hmm.. Re: talloc issues

Rusty Russell rusty at rustcorp.com.au
Mon Aug 3 19:20:12 MDT 2009


On Fri, 31 Jul 2009 05:44:13 pm Sam Liddicott wrote:
> Many libraries have their own free function that you have to use to free
> data that the library allocates. None of this is ever obvious to a
> casual user.

Yes, and this is one benefit of talloc; talloc_free is generic.

Note that this example *loses* that benefit.  Indirect free still works, but
direct talloc_free doesn't (caller must use talloc_unlink).

(I think this is still true after your changes, no?).

> 2. reference counting (which you don't have to use)
> #define malloc(s) talloc_zero(NULL, (s))
> #define ref_inc(s) talloc_reference(NULL, (s))
> #define ref_dec(s) talloc_unlink(NULL, (s))
> #define free(s) talloc_free((s))
> 
> and suddenly you can reference count all your pointers - even char*,
> with auto-freeing on zero refcount. Wow. Just like COM.

I think this is only a progression in your mind.  AFAICT references are
not inherent to talloc at all.  A variant can be built as a separate layer
on top of talloc (eg. http://ccan.ozlabs.org/info/talloc_link.html )

> When the problem gets more complicated than reference counting... then
> it's simpler to start reference counting.
> 
> ..and talloc is the easiest way to reference count - you don't have to
> change your structs for a start, and you don't have to port to C++ either.

I disagree.  To repeat myself, everyone needs to know you're using references,
even independent code, so they don't simply talloc_free() something.

Back to code.  Imagine a previous version of upcache which didn't cache:

	const char *get_upcase(const char *str)
	{
		char *ret = talloc_strdup(NULL, str);

		if (ret) {
			unsigned int i;
			for (i = 0; ret[i]; i++)
				ret[i] = toupper(ret[i]);
		}
		return ret;
	}

When we added caching, we now need to make sure noone talloc_free()'s this.
Tridge's "ban talloc_free" did this at runtime.  I think there's scope to
argue that this code should be changed completely to break at compile time
instead, (ie. return a "struct upcase" which is a real talloc object, and
does the string refcounting internally in the destructor).

Cheers,
Rusty.


More information about the samba-technical mailing list