[RFC PATCH] data_blob_realloc() accept 0 length argument

[Timur I. Bakeyev]

On 16 February 2018 at 10:34, Volker Lendecke <Volker.Lendecke at sernet.de>
wrote:

> On Fri, Feb 16, 2018 at 03:24:55AM +0100, Timur I. Bakeyev via
> samba-technical wrote:
> > Hi all!
> >
> > I was debugging one of our VFS modules and figured out that the code
> fails
> > when data_blob_realloc() receives a 0 as a length argument. For me,
> passing
> > 0, although seems to be useless, but still a valid invocation of the
> > function.
> >
> > Apparently, guys from the SSSD team also think so
> > https://pagure.io/SSSD/sssd/issue/2369.
> >
> > What do you think about similar fix for the data_blob_talloc()?
>
> Loooks good. Although I was under the impression that the talloc calls
> always return !=NULL, even with 0 length.
>

That was my impression as well , but apparently talloc_realloc() stands
aside from this pattern according to Andrew:

That matches the behaviour of talloc_realloc(), which is indeed for
>
realloc to zero length to be free.
>
> This can be confusing, as talloc() of zero length does an allocation,
> but that is the pattern.
>

This is definitely confusing, especially cause data_blob_realloc() doesn't
return DATA_BLOB*, which would make
it rough equivalent to talloc_realloc(), but just bool, making decisions
about validity of operation by itself.

So, may I propose at least such a fix, which would leave resulting
DATA_BLOB in a known state:

@@ -213,8 +213,10 @@ _PUBLIC_ DATA_BLOB data_blob_const(const
 _PUBLIC_ bool data_blob_realloc(TALLOC_CTX *mem_ctx, DATA_BLOB *blob,
size_t length)
 {
        blob->data = talloc_realloc(mem_ctx, blob->data, uint8_t, length);
-       if (blob->data == NULL)
+       if (blob->data == NULL) {
+               blob->length = 0;
                return false;
+       }
        blob->length = length;
        return true;
 }

 With regards,
Timur.