[trivial PATCH 11/22] [extern.h] eliminate build warnings
WANG Tinggong
wangtinggong at gmail.com
Thu Feb 4 17:32:48 CET 2010
Frank Cusack wrote:
>This applies to patches 11-16.
>
>Having these be individual unrelated patches is very annoying. It's
>very difficult to review because the change in prototype is not in
>the same change as the change to the functions. So I haven't done
>a thorough job, but for once I am going to blame the author instead
>of my own laziness.
>
>This just looks like churn to me. You change the types of vars from
>char to unsigned char, but then cast them back when they are used.
>
>In at least 2 cases where I caught it, the change in var type is
>incorrect and you are just making a trivial change without understanding
>the effects other than to quiet the compiler on your system.
Yes, that is my original intention. I clone the branch and compile it, after
my compiler show warnings to me, i just submit these patches without a moment's
thought and it's too impetuous.
>
>There may be at least one change that was correct, and that would only
>have been the case if a radius structure member changed its signedness.
>Because of the difficulty of reviewing the changes I'm not going
>to try to go back and pick out where I thought that might have
>been the case.
>
>I have in the past been extremely diligent about not having any
>compiler warnings so I'm surprised that rlm_otp is generating any
>noise. There must be one member in a radius struct somewhere that
>changed and that is filtering down.
In my opinion, a compiler warning may led to a potentional bug, for example,
int a = -1;
unsigned int b = 1;
(a > b) ? printf("yes") : printf("no");
then compiler tell us "warning: comparison between signed and unsigned",
if we ignore it, then -1 > 1, that's not expected. so i'm accustomed to fix all
warning.
thanks for your review!
Tinggong
More information about the Freeradius-Devel
mailing list