[Freeipa-devel] [PATCH] fix memleaks

[Simo Sorce]

On Fri, 2011-12-02 at 16:39 +0200, Alexander Bokovoy wrote:
> 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.

Ok pushed to master.

> > > 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.

Yes we are working on that with SSSD, if experimentation goes well we
will propose using gerrit for freeIPA too.

Simo.

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