[SCM] Samba Shared Repository - branch master updated
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
> > + * calls free() on the string options so it assumes the callers have
> > + * 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
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
> 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
so we can get this fixed in current shipping code.
More information about the samba-technical