[Freeipa-devel] [PATCH] fix memleaks

[Simo Sorce]

On Fri, 2011-12-02 at 16:04 +0200, Alexander Bokovoy wrote:
> On Tue, 29 Nov 2011, Simo Sorce wrote:
> > Found a couple of memleaks while reviewing code.
> > 
> > Attached.
> > 
> > Simo.
> > 
> > -- 
> > Simo Sorce * Red Hat, Inc * New York
> 
> > >From 70840691e48e1ac89002499c08a9dd4fdcae7c50 Mon Sep 17 00:00:00 2001
> > From: Simo Sorce <ssorce at redhat.com>
> > Date: Sun, 20 Nov 2011 20:50:11 -0500
> > Subject: [PATCH] ipa-kdb: fix memleaks in ipa_kdb_mspac.c
> > 
> > ---
> >  daemons/ipa-kdb/ipa_kdb_mspac.c |   13 +++++++------
> >  1 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
> > index cce1ca9060f0e03d525bb87d843bdd5811e9d20b..0c0da75ca8dbedf12e39e8ec5d87bfd4cd485d4a 100644
> > --- a/daemons/ipa-kdb/ipa_kdb_mspac.c
> > +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
> > @@ -466,7 +466,7 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
> >      TALLOC_CTX *tmpctx;
> >      struct ipadb_e_data *ied;
> >      struct ipadb_context *ipactx;
> > -    LDAPMessage *results;
> > +    LDAPMessage *results = NULL;
> >      LDAPMessage *lentry;
> >      DATA_BLOB pac_data;
> >      krb5_data data;
> > @@ -479,11 +479,6 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
> >          return KRB5_KDB_DBNOTINITED;
> >      }
> >  
> > -    tmpctx = talloc_new(NULL);
> > -    if (!tmpctx) {
> > -        return ENOMEM;
> > -    }
> > -
> >      ied = (struct ipadb_e_data *)client->e_data;
> >      if (ied->magic != IPA_E_DATA_MAGIC) {
> >          return EINVAL;
> > @@ -493,6 +488,11 @@ static krb5_error_code ipadb_get_pac(krb5_context kcontext,
> >          return 0;
> >      }
> >  
> > +    tmpctx = talloc_new(NULL);
> > +    if (!tmpctx) {
> > +        return ENOMEM;
> > +    }
> > +
> >      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.

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

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-simo-31e0-2-ipa-kdb-fix-memleaks-in-ipa_kdb_mspac.c.patch
Type: text/x-patch
Size: 1880 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111202/ed017c9f/attachment.bin>