[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