dom_sid_parse_length (was: Re: [PATCH] Cleanup patches)

[Jeremy Allison]

On Wed, Apr 29, 2015 at 11:24:24AM +1200, Andrew Bartlett wrote:
> On Tue, 2015-04-28 at 15:32 +0200, Volker Lendecke wrote:
> > From a76d79222518d239aa24d62ca205821ea02c14a6 Mon Sep 17 00:00:00 2001
> > From: Volker Lendecke <vl at samba.org>
> > Date: Fri, 2 Jan 2015 11:02:45 +0100
> > Subject: [PATCH 2/3] lib: Simplify dom_sid_parse_length
> > 
> > Signed-off-by: Volker Lendecke <vl at samba.org>
> > ---
> >  libcli/security/dom_sid.c |   11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
> > index 836979e..2910434 100644
> > --- a/libcli/security/dom_sid.c
> > +++ b/libcli/security/dom_sid.c
> > @@ -243,14 +243,9 @@ struct dom_sid *dom_sid_parse_talloc(TALLOC_CTX *mem_ctx, const char *sidstr)
> >  */
> >  struct dom_sid *dom_sid_parse_length(TALLOC_CTX *mem_ctx, const DATA_BLOB *sid)
> >  {
> > -       struct dom_sid *ret;
> > -       char *p = talloc_strndup(mem_ctx, (char *)sid->data, sid->length);
> > -       if (!p) {
> > -               return NULL;
> > -       }
> > -       ret = dom_sid_parse_talloc(mem_ctx, p);
> > -       talloc_free(p);
> > -       return ret;
> > +       char p[sid->length+1];
> > +       memcpy(p, sid->data, sizeof(p));
> > +       return dom_sid_parse_talloc(mem_ctx, p);
> >  }
> 
> Can you please explain to me how this works?  
> 
> As I see it, we now read 1 byte beyond the DATA_BLOB length, copy that,
> and then presume this uninitialised value is a NULL terminator for
> dom_sid_parse_talloc(), but without explicitly setting it.
> 
> What am I missing?

Oh, I reviewed that. Sorry, looks like I missed that :-(.