Hacker News Reader

Comment by irishcoffee | Hacker News Reader

This is really neat, I'm glad it worked.

This is atrocious C code.

Plus zig!

  **Decision**: Use C for kernel interactions, Zig for pure logic only.

https://github.com/narqo/freebsd-brcmfmac/blob/be9b49c1bf942...

https://github.com/narqo/freebsd-brcmfmac/blob/be9b49c1bf942... seems to be the only file written in Zig.

Using an entire additional programming language for 229 lines of code is definitely an interesting choice.

Looks fairly idiomatic. What specifically do you dislike about it?

Got a sample you think is particularly bad?

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.

I'm no expert either and I have not done kernel development, but I've done some embedded stuff in C and I think this is not unreasonable. brcmf_reg_read is only called in one place and the call chain is straightforward (starts in pcie.c brcmf_pcie_attach). Its always initialized by device_get_softc (https://man.freebsd.org/cgi/man.cgi?query=device_get_softc&s...) so as long as the device is attached its initialized. Likely something fails much earlier if it is not attached. I think this is pretty typical for low-level C, it would definitely not be idiomatic for every function in this call chain to check if sc was initialized - I don't know if there is a need to check it after calling device_get_softc but that would probably be reasonable just so people reviewing it don't have to check.

Some application code bases I've worked in would have asserts in every function since they get removed in release builds but I don't know that debug builds are even a thing in kernel devices.

To be fair if you open up driver source code from the vendors themselves, it's often the same hell with magic numbers and lack of checks because "we know what the hardware will return". But you're right on the main point: AI writes C like a very confident junior who skipped memory safety lectures - it copies the style, but not the discipline. It works as long as you're on the "happy path", but debugging a kernel panic in code like that is going to be painful

I was personally surprised when the agent debugged kernel panics caused by its own code (many times by now). It just iterates from the stack traces and crash dumps. The nice part is that, when you do see that the code smells — you ask the agent to rework it, focusing on specific problems. This is just code, and you don't need to dance around, hoping that AI will spill some "magic" at you.

Dumping panic traces to an agent works fine if it's just a vanilla page fault at an obvious address. But when your memory gets corrupted by some scuffed DMA sync or a race condition in an interrupt handler, the kernel panics a million clock cycles after the actual bug occurred. The dump is just pure garbage by then, and no LLM is going to untangle it because the root cause context literally isn't in the logs tbh

> The nice part is that, when you do see that the code smells — you ask the agent to rework it, focusing on specific problems.

I think that is the crux of the problem. How do you know code smell if you don't write it, and you don't read it? I'm pretty confident the spdx header isn't correct even.

Above it was said, that in a code review, an expert would ask the author to "justify and rework". Clearly, people have always been capable producing code, that wasn't great, regardless if they read the code or not.

Reply on news.ycombinator.com