Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo


#7 Error handling in etxack.c

vrflash (2)

Hello. Debian bug report #177297
(http://bugs.debian.org/177297) shows some strangeness
in the error handling in ex_sendfile() in etxack.c.

I see three problems in it:

1. If the result of the read() is not 1, the function
prints an error message that uses the value of c, which
will not have been initialized if the read failed.

2. The function compares c to EOF. However, c is a
char (which might be either signed or unsigned,
depending on the architecture) and EOF is defined as
-1. This check will never match. Even if c is signed,
the preceding parity mask will have limited its range
to [0 .. 127].

3. The code that should handle read failures (i.e.
checking for EAGAIN and reporting a read error) is in
this c == EOF branch, where it
a. will never get executed, and
b. doesn't have a valid errno value anyway.

I made a patch to fix things up, but I don't know it
will actually work, because I no longer have a working
Agenda myself. The patch does these things:

1. Add a ret < 0 check to the EAGAIN check, to make
sure we have a valid errno.

2. Move that error handling from the c == EOF branch
to the ret != 1 branch, including the return -1 so that
the rest of the loop can rely on c containing valid
data. Note that this gives tet == 0 (end of file) the
same message as ret < 0 (read error), which seems
appropriate in this context. I don't think serial
ports are supposed to have EOF conditions.

3. Move the "read '0x%x' instead of ACK" message to
the end of the loop, where it gets printed if c was
neither ACK nor NAK.

4. Delete what's left of the c == EOF case.

Please let me know if this patch breaks anything :)
I'll add it to the Debian package to fix that
bugreport, but I don't know if anyone is using the
Debian package. (I plan to drop this package from
Debian in a few months.)


  • Error handling patch for etxack.c.