[PATCH] group_mapping: Avoid a talloc

[Andrew Bartlett]

On Wed, 2014-01-08 at 11:44 -0700, Christof Schmitt wrote:
> On Wed, Jan 08, 2014 at 10:59:03AM +0100, Volker Lendecke wrote:
> > Hi!
> > 
> > Please review & push!
> > 
> > Thanks,
> > 
> > Volker
> > 
> > -- 
> > SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> > phone: +49-551-370000-0, fax: +49-551-370000-9
> > AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> > http://www.sernet.de, mailto:kontakt at sernet.de
> 
> > From d77f1d4eb1a102975ee46328e559740b5b1248db Mon Sep 17 00:00:00 2001
> > From: Volker Lendecke <vl at samba.org>
> > Date: Tue, 3 Dec 2013 16:01:35 +0100
> > Subject: [PATCH] group_mapping: Avoid a talloc
> > 
> > Signed-off-by: Volker Lendecke <vl at samba.org>
> > ---
> >  source3/groupdb/mapping_tdb.c |   14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> > 
> > diff --git a/source3/groupdb/mapping_tdb.c b/source3/groupdb/mapping_tdb.c
> > index 088874f..05b6fce 100644
> > --- a/source3/groupdb/mapping_tdb.c
> > +++ b/source3/groupdb/mapping_tdb.c
> > @@ -119,17 +119,15 @@ static bool init_group_mapping(void)
> >  
> >  static char *group_mapping_key(TALLOC_CTX *mem_ctx, const struct dom_sid *sid)
> >  {
> > -	char *sidstr, *result;
> > +	char sidstr[DOM_SID_STR_BUFLEN];
> > +	int len;
> >  
> > -	sidstr = sid_string_talloc(talloc_tos(), sid);
> > -	if (sidstr == NULL) {
> > -		return NULL;
> > +	len = dom_sid_string_buf(sid, sidstr, sizeof(sidstr));
> > +	if (len >= sizeof(sidstr)) {
> > +		return false;
> 
> This should get changed back to return NULL to match the return type.
> 
> Everything else looks good, so with this change:
> Reviewed-by: Christof Schmitt <cs at samba.org>

I can see the argument for stuff in tight inner loops, but I'm very,
very wary of going back to fixed length strings everywhere.  What's the
whole argument of talloc_pool() that this was fast enough, without all
the risks of fixed length strings?

There is a great value in consistency of talloc-based string handling,
because it means that we can realistically examine our fixed length
string use, because it's rare, not used anywhere we might save a cycle
or two outside the hot path.

In short, I don't like where I see this headed, and I think we should
consider that.

Sorry,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba