On Tue, 2005-06-21 at 20:06 -0400, [EMAIL PROTECTED] wrote:
> Andrew Tridgell <[EMAIL PROTECTED]> 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    -  [EMAIL PROTECTED]
Samba Team    -  http://www.samba.org
Italian Site  -  http://samba.xsec.it

Reply via email to