PATCH for Coverity issues

Simo simo at samba.org
Thu Feb 20 12:54:34 MST 2014


On Thu, 2014-02-20 at 19:34 +0530, Santosh Pradhan wrote:
> Hi Simo/Volker,
> I replaced free() with krb5_free_default_realm() to deallocate the 
> memory allocated by krb5_get_default_realm().
> 
> Attached the patch for review.
> 
> Best Regards,
> Santosh
> 
> On 02/20/2014 06:52 PM, Simo wrote:
> > On Thu, 2014-02-20 at 18:49 +0530, Santosh Pradhan wrote:
> >> Thanks for the quick reply.
> >>
> >> On 02/20/2014 06:26 PM, Volker Lendecke wrote:
> >>> On Thu, Feb 20, 2014 at 06:21:10PM +0530, Santosh Pradhan wrote:
> >>>
> >>>> What about the 2nd patch ?
> >>> For this one people more familiar with the krb5 libraries
> >>> should comment. I'm not sure a plain "free" here is the
> >>> right thing. There's a ton of "krb5_free_something" routines
> >>> used, and I don't know if there's a "krb5_free_realm"
> >>> function or "krb5_free_string" that would be more
> >>> appropriate in this context.
> >> Yes, I was thinking of using krb5_free_default_realm() which takes a
> >> context param (never used) along with realm param. Internally it
> >> callskrb5_xfree() which is nothing but free(). But I found plain free()
> >> is also used  e.g. in smb_krb5_make_principal() , FILE:
> >> lib/krb5_wrap/krb5_samba.c.
> > Please use krb5_free_default_realm() and fix the other place where
> > free() is used directly too.
> >
> > Simo.
> >
> 

Hi Santosh,
the patch looks good, however our policy is to confine changes to the
heimdal code in a separate patch than pother changes so we can more
easily send those patches upstream.

Could you please split them ?

Besides this technicality you can add my Reviewed-by: Simo Sorce
<idra at samba.org>


Simo.



More information about the samba-technical mailing list