[Freeipa-devel] [PATCH] cleanups

[Rich Megginson]

Dmitri Pal wrote:
> Simo Sorce wrote:
>   
>> To all,
>> I have done janitorial work to fix lots and lots of warnings (mainly in
>> common/collection) in the following 2 patches.
>>
>>   
>>     
> Thank you. But why they do not show up automatically in the normal build?
> I trust whatever Steven has setup as the default build and I do not see
> any warnings otherwise I would have addressed them.
>   
CFLAGS to do warnings, etc. are traditionally passed in by the build 
system.  For example, these are the flags used when you use rpmbuild on 
most of our platforms:
rpm --eval '%{optflags}'
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic

You'll notice the -Wall there - that causes many warnings to be printed.
>   
>> We have only 1 warning left in collection (I will need to discuss this
>> one with Dmitri next week).
>>
>>   
>>     
> I am just used to checking free(). In old times it was dangerous to free
> without checking.
> I do not know how porting to other platforms would go down the road.
> It might be that we would have to use other compiler on other platforms
> and it might not be as smart.
> Also I suspect that smart compiler probably optimizes the code so the
> duplicate checking is removed.
> It is a bit of defensive programming. If you think it is not worth
> checking nowadays I will stop doing it.
>   
There was a discussion about this a while ago on tech-list, and also in 
the mozilla newsgroups.  It has been at least 10 years since checking 
for NULL before free was recommended.  It is not needed anymore on any 
platform.
>
>   
>> Please,
>> before submitting a patch do a complete build from scratch using the
>> following command:
>>
>> make CFLAGS="-Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
>> -Wcast-qual -Wcast-align -Wwrite-strings" LIBTOOLFLAGS=--silent --quiet
>>
>>   
>>     
> This should be baked into the configure so it can be easily turned off
> and on (if not be on by default).
>   
It's tricky to do that portably.
>
>
>   
>> If you see any output but "Making all in <subsystem>" then you have a
>> bug to fix before sending the patch in for review.
>>
>> cheers,
>> Simo.
>>
>>   
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>     
>
>
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3258 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090706/2ff97da8/attachment.bin>