svn commit: samba r7759 - in branches/SAMBA_4_0/source/lib/ldb/common: .

[Simo Sorce]

On Tue, 2005-06-21 at 20:06 -0400, derrell at samba.org wrote:
> Andrew Tridgell <tridge at osdl.org> writes:
> 
> > Derrell,
> >
> >  > Shouldn't this be talloc_strdup("ldb not connected")?  The error string from
> >  > some modules is likely to have to be allocated, as it may be coming,
> >  > initially, from an external library which malloc()s it, so the string will
> >  > have to be talloc_strdup()ed to be returned. (e.g. sqlite).
> >
> > No. The caller does not free in this API. It's fine for a backend to
> > allocate the string, in which case the backend will need to do
> > something like this:
> >
> >   talloc_free(mybackend->last_error_string);
> >   mybackend->last_error_string = talloc_strdup(mybackend, "a new error");
> >   return mybackend->last_error_string;
> 
> That's kinda yucky.  (No, that's really yucky.)  Is there some reason that you
> wouldn't want the caller to just free the error string,

The reason is that the caller is not obliged to read (and free) the
error message on an error, so that would make, either the caller code
too heavy and unreadable, or it will leak memory.

>  or better yet, for the
> error string to always just be allocated on the ldb context so that it gets
> freed automagically?

it would get freed at he ldb close, that may be thousand of operations
and erros later, that would make a huge memleak.

>   With everything so clean in ldb, this seems like a real
> oversight.

I think that system is good enough right now, others have worse problems
imho.

Simo.

-- 
Simo Sorce    -  idra at samba.org
Samba Team    -  http://www.samba.org
Italian Site  -  http://samba.xsec.it