logoalt Hacker News

cryptonectoryesterday at 11:06 PM1 replyview on HN

Got a sample you think is particularly bad?


Replies

irishcoffeetoday at 12:07 AM

Throwing myself to the sharks here, but sure.

pcie.c

  /*
  * BAR0 register access
  */
  uint32_t
  brcmf_reg_read(struct brcmf_softc *sc, uint32_t off)
  {
 return (bus_space_read_4(sc->reg_bst, sc->reg_bsh, off));
  }
Is sc null? Who knows! Was it memset anywhere? No! Were any structs memset anywhere? Barely! Does this codebase check for null? Maybe in 3% of the places it should!

All throughout this codebase variables are declared and not initialized. Magic numbers are everywhere AND constants are defined everywhere. Constants are a mix of hex and int for what seem to be completely arbitrary reasons. Error handling is completely inconsistent, sometimes a function will return 5 places, sometimes a function will set an error code and jump to a label, and sometimes do both in the same function depending on which branch it hits.

All of this is the kind of code smell I would ask someone to justify and most likely rework.

Or I'm just a dumbass, I suppose I'll find out shortly.

show 1 reply