[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Fri Jan 15 18:21:48 MST 2010
On Fri, Jan 15, 2010 at 08:07:21PM -0500, Derrell Lipman wrote:
> On Fri, Jan 15, 2010 at 19:45, Jeremy Allison <jra at samba.org> wrote:
>
> On Thu, Jan 14, 2010 at 12:50:26PM -0600, G nther Deschner wrote:
>
> > +/* note the strdup for string options on smbc_set calls. I think
> libsmbclient is
> > + * really doing something wrong here: in smbc_free_context libsmbclient
> just
> > + * calls free() on the string options so it assumes the callers have
> malloced
> > + * them before setting them via smbc_set calls. */
>
> God you're right - that's *REALLY* horrible. libsmbclient
> should have been deep copying the strings from the start,
> not just stealing the pointers.
>
>
>
> I missed the beginning of this conversation. The message including Gunther's
> above comment didn't arrive here.
It was a git commit message.
> I'm looking at smbc_free_context for free() calls to see what this is referring
> to. I see that I free the return value of smbc_getWorkgroup(), etc, thus the
> smbc_set calls it refers to would be the like of smbc_setWorkgroup(). I'm
> pretty sure that these were previously simple assignments before I added the
> set/get interface, so it's "always" been this way, with the assumption that
> those values are malloc'ed. That not withstanding, certainly, if that was the
> assumption, it would have been nice if it had been documented it in
> libsmbclient.h.
Yes, I'm sure it's one of these things that have "always" been that
way, but IMHO we really need to fix this asap. I know it might cause
a memory leak in code that currently uses it "correctly" but I can't
find any code out there that does do this correctly (except Guenthers
test code) so I don't think this will be a problem. It's so
counter-intuitive !
> Converting this to a deep copy is perfectly reasonable, with the caveat that
> code expecting their pointer to be freed will end up with a memory leak if not
> otherwise changed.
I'm going to commit a patch to master that does this, with an associated
bug:
https://bugzilla.samba.org/show_bug.cgi?id=7045
so we can get this fixed in current shipping code.
Jeremy.
More information about the samba-technical
mailing list