[Freeipa-devel] [PATCH] cleanups
Simo Sorce
ssorce at redhat.com
Mon Jul 6 12:14:45 UTC 2009
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
More information about the Freeipa-devel
mailing list