Re: [PATCH v2] malloc: Add posix_memalign test.

["Carlos O'Donell" <carlos at redhat dot com>]

On 10/02/2013 12:57 PM, Adhemerval Zanella wrote:
> On 02-10-2013 13:45, Carlos O'Donell wrote:
>> Will,
>>
>> Thanks for the additional test case.
>>
>> Some comments below, there are some things that
>> should get fixed up.
>>
>>> +  if (ret == ENOMEM && p != NULL)
>>> +    merror ("returned an error but pointer was modified");
>>> +
>> OK.
>>
>> Needs free (p).
> 
> From my point of view, there is no need in calling free on a NULL ponter. In fact
> free(NULL) will only call the malloc hook, if any, and still in this case I don't
> see it needed. It also aplies for other cases where you require the free().
 
If p is non-null then posix_memalign allocated
something.

The standard then says:
~~~
The free() function shall deallocate memory that
has previously been allocated by posix_memalign()
~~~

So we might as well test `free (p)' also works
correctly for each test?

It's beneficial IMO to go through the whole sequence
of calls as the user would also and that includes
calling free even if p is NULL to exercise the hook
code.

>>
>>> +  ret = posix_memalign (&p, sizeof (void *), 0);
>>> +
>>> +  if (ret != 0 || p == NULL)
>>> +    merror ("posix_memalign (&p, sizeof (void *), 0) failed.");
>>> +
>>> +  free (p);
>> OK.
>>
>>> +
>> Test case needs comment e.g. 
>> /* Test that a multiple of sizeof(void *) but not a power of two, fails.  */
>>
>> Should set p to NULL (for later free).
> 
> Also, posix_memalign disconsider the value of '*memptr', so I also think it is not needed
> to really sets it to NULL.

Could you please clarify what you mean by `disconsider?'

Do you mean to say *memptr is set to NULL?

I was worried it might be left unmodified and that's problematic.

Cheers,
Carlos.