CodeSOD: The Serial Killer

Debugging code on embedded devices introduces additional challenges to tracking down what's going wrong, especially as in many cases, you can't even rely on any sort of println debugging. I've worked with a few devices which my only debugging output is 3 RGB LEDs.

Serial devices, then, can be a godsend. For short runs, you can often just wire pin-to-pin, but for longer connections, you need some sort of serial transceiver chip. In most practical situations, the flow between two devices is CPU->UART->transceiver->cable->transceiver->UART->CPU. Apologies to the OSI model, yes, it's more complicated than this, but for our purposes, it's a useful illustration.

Stephen was debugging a board where the chip supported four UARTs, but the board had two different transceivers on it, each responsible for two different UARTs. What was supposed to happen was that when data arrived via the serial bus, an interrupt would fire and react to the message. None of that was happening.

It wasn't hard to find the method responsible for setting up the interrupts:

 void sysSerialHwInit2 (void) { int i; /* connect serial interrupts */ for (i=0;i<N_UART_CHANNELS; i++) if (chipOneChan[i].int_vec) { (void) intConnect (INUM_TO_IVEC (chipOneChan[i].int_vec), (VOIDFUNCPTR)chipOneInt, (int)&chipOneChan[i] ); sysIntEnablePIC (chipOne_devParas[i].intLevel);for (i=0;i<N_CHIPTWO_CHANNELS;i++){ (void) pciIntConnect(INUM_TO_IVEC (INT_NUM_GET (chipTwoChan[i].level)), (VOIDFUNCPTR)chipTwoInt, (int)& chipTwoChan[i]); } sysIntEnablePIC (chipTwoChan[i].level); } }

The whitespace is as originally found, and it did an exceptional job of hiding the bug. If you just glance at this code, trying to understand its flow from its formatting, it seems pretty sane: set up the interrupts for our UART channels, and then set up the interrupts for our "chiptwo" channels.

Of course, that's not what's actually happening. "Whitespace matters," writes Stephen, and after he let his editor fix the whitespace, the actual flow of the code became clear:

 void sysSerialHwInit2 (void) { int i; /* connect serial interrupts */ for (i = 0; i < N_UART_CHANNELS; i++) if (i8250Chan[i].int_vec) { (void) intConnect (INUM_TO_IVEC (i8250Chan[i].int_vec), (VOIDFUNCPTR)i8250Int, (int)&i8250Chan[i] ); sysIntEnablePIC (i8250_devParas[i].intLevel); for (i = 0; i < N_XR17D_CHANNELS; i++) { (void) pciIntConnect(INUM_TO_IVEC (INT_NUM_GET (xr17dChan[i].level)), (VOIDFUNCPTR)xr17dInt, (int)& xr17dChan[i]); sysIntEnablePIC (xr17dChan[i].level); } } } 

A few missing curly braces meant that what was supposed to be two separate for loops were mashed into one for loop which didn't work.

It wasn't hard to fix the curly braces and the whitespace. It was harder to talk the other developers on the team into setting up their editors to use a consistent approach to whitespace.

[Advertisement] Forget logs. Next time you're struggling to replicate error, crash and performance issues in your apps - Think Raygun! Installs in minutes. Learn more.