Re: [PATCH] Allocation buffers for NSS result construction

[Florian Weimer <fweimer at redhat dot com>]

On 05/16/2017 01:32 AM, DJ Delorie wrote:
> I have no specific comments on this, it looks pretty good and I like the
> idea of centralizing this (I just did something similar in the NSS
> testsuite, could have used this).  I mostly focused on the allocation
> routines; I'm not familiar with resolv yet.

Thanks for your comments.

> I do worry about having "internal" (internal to the allocator, not
> internal to glibc) functions in a header.  They're written as if they'll
> never be called by "outsiders" (i.e. little argument checking) but
> there's no protection against such calls.  I don't know how to add such
> protection, though.

We could use #pragma GCC poison.  I'll give it a try.

> Not having a pointer to the buffer's beginning irked me a bit, but I
> don't see any reason to include it anyway.

It's important if you want to flip an output buffer to an input buffer 
(which covers the parts of the buffer that has been written).  I think 
it could make sense to add those as a separate abstraction eventually.

> I would recommend a few comments defining the behavior if NULL is passed
> as a thing to copy - the NSS testsuite needed to preserve such
> "mistakes" in the test data.

Hmm.  It's undefined because of the memcpy.  I'm not sure if I want to 
change that, it would need a mostly pointless pointer comparison before 
the memcpy call.

> I wonder if this new functionality is complex enough to warrant a
> separate bit of documentation?  Either in the manual or elsewhere...

Right.  We actually have internal documentation for NSS in the manual. 
We could add to that (even though the facilities are only available 
within glibc itself).

> The reallocarray patch also includes a __check_mul_overflow_size_t style
> function; this duplication will need to be resolved as patches get
> committed.

Right, it's on my radar.

Thanks,
Florian