[En-Nut-Discussion] Bugs in Uart.c

[Nathan Moore]

>
> 1: UsartRead
> if (NutEventWait(&rbf->rbf_que, dcb->dcb_rtimeout)) {
> should be
> if (NutEventWait(&rbf->rbf_que, dcb->dcb_rtimeout) == -1) {

The first one is correct since it is the same as
  NutEventWait(&rbt->rbf_que, dcb->dcb_rtimeout) != 0
If the return value is 0, the first conditional will not be executed, but if
the return value is -1 it will be with that comparison.  It will also be
executed if the return value is some other nonzero value, but that never
happens.
The return value is used as a boolean, so the first comparison usually
produces better code because just about all processors have better detection
of == 0 (and therefore != 0) than other comparisons which often involve
inlining or loading the compare to value in order to do the compare, which
is bigger code.

>
>
> 2: Still UsartRead:
> rc should be unsiged int. The is no reason to make it size_t as it will
> never become bigger then size. The disadvantage is, that for the avr
> size_t is an unsigned long, which costs performance.

size_t is defined to an archetectually dependant type to specify which can
describe the size of any chunk of data.  It's probably already equavelent to
an unsigned int on your architecture.


>
>
> 4: Once again the return types are often much bigger then needed. This
> costs registers and performance.

I agree that the types are often bigger than needed.  NutEventWait uses an
extra byte; the biggest impact of this is that it would cause every caller
who checks the result to have an extra instruction to check each byte on an
AVR.  It should return some sort of archetectually specific boolean type,
which would be a char (or unsigned char) for AVR or an int for other
architectures.

Nathan