you are viewing a single comment's thread
view the rest of the comments
[–] 7 points 1 year ago (9 children)

I don't know, when you are dealing with this level of coding it should be top tier, and getting called out will make the person really review their next submission. The expectation that somebody always has to be nice to you while you fuckup, is not ideal. And I say that as a supervisor that is way too PC and nice to people whom unwittingly are sabotaging work, as a way to nurture them--but I honestly think it is counter productive

  • source
  • parent
  • hideshow 9 child comments
  • [–] 9 points 1 year ago (7 children)

    getting called out will make the person really review their next submission.

    Yeah or they'll say "fuck this" and quit.

    The expectation that somebody always has to be nice to you while you fuckup, is not ideal.

    It's hardly a fuck up. They named a function slightly poorly. As if Linus has never done that.

  • source
  • parent
  • hideshow 7 child comments
  • [–] 12 points 1 year ago (5 children)

    Cmon, this is not about naming, this is about non-generic code in generic header.

    IMO hiding such a little operation behind a macro/function just hurt readability. Furthermore, considering that this function is only used once in the provided patch and that word ordering on RISC-V is not about to change anytime soon, it is perfectly fine to inline the code.

  • source
  • parent
  • hideshow 5 child comments
  • [–] 0 points 1 year ago (4 children)

    this is about non-generic code in generic header.

    (a << 16) | b is about the most generic code you can get. How is that remotely RISC-V specific?

  • source
  • parent
  • hideshow 4 child comments
  • [–] 11 points 1 year ago* (last edited 1 year ago) (3 children)

    Making a u32 pointer from two u16's isn't a generic operation because it has to make assumptions about how the pointers work endianess

    Edit: Actually, I'm wrong, didn't think this through properly. See the replies

  • source
  • parent
  • hideshow 3 child comments
  • [–] 2 points 1 year ago (2 children)

    What makes you think it's making a pointer? Nobody said anything about that.

  • source
  • parent
  • hideshow 2 child comments
  • [–] 2 points 1 year ago (1 child)

    Oh my bad I don't know where I got that from lol

  • source
  • parent
  • hideshow 1 child comment
  • [–] 3 points 1 year ago

    Nw. You're also wrong about endianness. This function would be written exactly the same irrespective of endianness:

    uint32_t u16_high_low_to_u32(uint16_t high, uint16_t low) {
      return (high << 16) | low;
    }
    

    That is endian agnostic.

  • source
  • parent