[Freeipa-devel] [PATCH] fix memleaks

Alexander Bokovoy abokovoy at redhat.com
Fri Dec 2 14:39:31 UTC 2011


On Fri, 02 Dec 2011, Simo Sorce wrote:
> > >      memset(&pac_info, 0, sizeof(pac_info));
> > >      pac_info.logon_info.info = talloc_zero(tmpctx, struct PAC_LOGON_INFO);
> > >      if (!tmpctx) {
> > Here is an issue -- you are allocating off tmpctx which is not empty 
> > here (we checked it right above) but then you are checking tmpctx 
> > rather than pac_info.logon_info.info.
> > 
> > It is an older error but needs to be fixed as well.
> 
> Ok added this fix.
ACK.

> > Also please name the patch file according to 
> > https://fedorahosted.org/freeipa/wiki/PatchFormat :)
> 
> I lost count of how many patches I handled, and to be honest I think
> this naming convention sucks a bit.
> 
> I would be ok changing the number to match a ticket number perhaps, when
> there is a ticket attached to it, but for patches like this one all you
> really-need it to append a -2 at the end if a new revision is necessary.
> Putting arbitrary numbers and names, doesn't make it any easier to
> handle to me.
I'm fine with any way that does not introduce file names conflicts.
We can even write a simple script that takes information from the 
patch itself and renames the files in a directory according to the 
established practice.

But I think better solution would be to use Gerrit to handle patch 
reviews. Then the issue of patch names will go away as you'll be 
dealing with git pull/push mostly.

-- 
/ Alexander Bokovoy




More information about the Freeipa-devel mailing list