[Qemu-devel] Re: completely OT: C Q/As, was Re: security_20040618

[Charlie Gordon]

"Michael Jennings" <address@hidden> wrote in message
news:address@hidden
> There are two problems with using enum's and/or #define's for
> TRUE/FALSE.  They should not be used in boolean expressions except
> where the return value is of that same typedef (e.g., the function
> returns bool) or is generated from the same set of #define's.  While
> false is always 0, true is not always 1.  True is non-zero.

This is a common misunderstanding.  your last statement doesn't make sense :
in C, false IS 0 and true IS 1.  That is the result of a false comparison is
0 (eg: 2 == 3) and the result of a true comparison is 1 (as in 2 == 2).  As
such boolean expressions can only have 2 values : 0 and 1, appropriately
defined as FALSE and TRUE, be it via #define or enum.
Unlike its distant cousin Java, C allows non boolean expressions to control
test statements such as 'if', 'for', and 'while'.  In such constructs,
falseness is not limited to the integer value 0 : '\0', 0.0, and NULL
pointers also convert to false, while all other values convert to true.
Further confusion is caused by the convention used in the standard C library
where boolean valued function do not necessarily return 1 for trueness.
Furthermore, it is perfectly OK to compare values from different typedefs
but identical implementation types.
I do agree with you that there is room for confusion, especially for java
bred programmers, and it is usually better to avoid the extra clutter of
comparing to FALSE or TRUE in test expressions : just use the plain
expression or bang (!) it for the reverse condition.

> Forgot "static" before char x[20];, or to be more threadsafe, either
> malloc() or pass in a buffer and max size.

These are obvious solutions to the problem, the latter (buffer and size) is
actually the one used in the C library for that very function (itoa).  But
the question was a different one : Why does this crash later (if you are
lucky) ?

> > defensive programming would require that TRUE be also defined as
> >
> > #define TRUE 1
> >
> > as many unsuspecting programmers will expect TRUE and FALSE to be
handled in
> > the preprocessor phase eg:
> >
> > #if TRUE
> >     ...
> >     somecode();
> >     ...
> > #endif
>
> I disagree strongly.  Anyone who writes "#if TRUE" or "#if FALSE" is
> just asking for trouble; (s)he needs to be taught a lesson.  "#if 0"
> and "#if 1" are just as obvious, and both "#if 1" and "#if TRUE" are
> equally ridiculous.

You use #if 0 yourself, try pretending you never switch those to #if 1 ?
I used to be harsh like this and show contempt for imperfect programming.
I've had to grow more tolerant, as 99% of C or C++ code is indeed imperfect,
and most programmers are more focussed on problem solving than clean coding.
The truth is that *anyone* who writes C code is just asking for trouble.
Yet this is still my favorite language.  I do use #if 0 or #if 1 sometimes,
never #if TRUE or #if FALSE, but I do not believe people that do should be
taught such a difficult lesson : after all, it is an accepted convention
that all uppercase identifiers be preprocessor defined. Christof Petig
provided a more compelling example :

#define USE_MORE_CAUTION  TRUE
#if USE_MORE_CAUTION == TRUE
    // this code would compile
#endif
#if USE_MORE_CAUTION == FALSE
    // this code would compile as well
#endif

My point is that if you define TRUE and FALSE in a header file, your job is
to make sure that you are not setting up boobytraps for the unsuspecting
(and imperfect) programmer to trigger.

I took a look at libast and you DO redefine TRUE and FALSE so as to setup
such a trap !!! have pity on your users!

/* from libast/types.h */
...
#undef false
#undef False
#undef FALSE
#undef true
#undef True
#undef TRUE

typedef enum {
#ifndef __cplusplus
  false = 0,
#endif
  False = 0,
  FALSE = 0,
#ifndef __cplusplus
  true = 1,
#endif
  True = 1,
  TRUE = 1
} spif_bool_t;
...

While your code is amazingly consistent and readable (except for line
length), you can learn from this very thread and fix it : you use strncpy 10
times. 7 of those are incorrect and at least 5 will lead to potential buffer
overflow and inefficiencies, the remaining 3 could be replaced directly with
calls to memcpy.  You make one incorrect use of strncat, that may overflow
by just one byte.  pstrcpy provides the very semantics you require in most
of these cases.

Cheers,

Chqrlie.