[ovs-dev] [PATCH 1/4] util: Disallow zero-sized xmalloc_cacheline
yamamoto at valinux.co.jp
Mon May 5 23:42:05 UTC 2014
> On Mon, May 05, 2014 at 08:32:34AM +0900, YAMAMOTO Takashi wrote:
>> > On Sat, May 03, 2014 at 09:01:01AM +0900, YAMAMOTO Takashi wrote:
>> >> xmalloc_cacheline API is relatively new. It's better
>> >> not to inherit the kludge from xmalloc. This kind of
>> >> kludge rather hurts these days.
>> >> Signed-off-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
>> > I see basically three alternatives for xmalloc(0) and
>> > xmalloc_cacheline(0):
>> > 1. Assert-fail.
>> > 2. Return NULL.
>> > 3. Return a unique 1-byte block.
>> > I'm not a big fan of #1 because it can create corner cases where one
>> > must be extra careful, mainly when one is allocating a variable-length
>> > array that might occasionally have zero elements.
>> > #2 and #3 have about the same effect most of the time. Since
>> > dereferencing the pointer returned by #3 yields undefined behavior
>> > according to the C standard, there isn't much of an advantage to #3 over
>> > #2. The only practical difference is that occasionally a nonnull
>> > pointer indicates that some data structure has been initialized.
>> > I've always leaned toward #2, as a personal opinion, but I went with #3
>> > in Open vSwitch xmalloc() because of my GNU background, since GNU code
>> > has a bias toward malloc(0) acting like malloc(1). (gnulib goes so far
>> > as to test for malloc(0) behavior and add a wrapper if it returns NULL.)
>> > So my preference is #2 or #3, leaning toward #3 since it's the behavior
>> > we've had in OVS for a long time. To me, #1 seems risky: it makes a
>> > rare corner case definitely deadly.
>> thanks for explaining your reasoning.
>> i can understand it, although i still prefer failing loudly rather
>> than silently.
> I guess I don't see an allocator that returns a 0-byte block in response
> to a 0-byte request as "failing".
however, the calling code might fail with the 0-byte block
if it was not written carefully enough.
>> i will drop this patch for now as it seems at least controversial.
More information about the dev