[Freeipa-devel] [PATCH] cleanups

[Simo Sorce]

On Mon, 2009-07-06 at 10:36 +0200, Sumit Bose wrote:
> On Fri, Jul 03, 2009 at 12:48:22PM -0400, 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.
> > 
> > We have only 1 warning left in collection (I will need to discuss this
> > one with Dmitri next week).
> > 
> > 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
> > 
> > If you see any output but "Making all in <subsystem>" then you have a
> > bug to fix before sending the patch in for review.
> > 
> 
> ACK
> 
> But I have two more questions:
> 
> - I still get 'collection.c:72: warning: initialization discards
>   qualifiers from pointer target type' because dummy_item is initialized
>   with "" which is const. While looking at col_delete_collection I think
>   it might be possible that a free is called on that "". Shouldn't we
>   use a strdup'ed "" to initialize dummy_item to be on the safe side?

Yes, this is exactly the thing I said I need to talk Dmitri about.

> - some people recommend to replace code like:
>     if (item->property != NULL) free(item->property);
>   by
>     free(item->property);
>   (see
> http://post-office.corp.redhat.com/archives/tech-list/2009-May/msg00659.html)
> 
>   Shall we try to meet this recommendation and change this? (Yes, I know
>   I have done this in pam_sss.c, too. :-)

Yes, absolutely, it makes not sense to check for NULL, free(NULL) is
just a noop, worth not wasting checking about it.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York