This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH v2] malloc: Add posix_memalign test.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 02 Oct 2013 13:06:08 -0400
- Subject: Re: [PATCH v2] malloc: Add posix_memalign test.
- Authentication-results: sourceware.org; auth=none
- References: <523042B4 dot 90602 at linaro dot org> <524C4D9E dot 3070701 at redhat dot com> <524C5073 dot 3010402 at linux dot vnet dot ibm dot com>
On 10/02/2013 12:57 PM, Adhemerval Zanella wrote:
> On 02-10-2013 13:45, Carlos O'Donell wrote:
>> 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");
>> 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
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
>>> + ret = posix_memalign (&p, sizeof (void *), 0);
>>> + if (ret != 0 || p == NULL)
>>> + merror ("posix_memalign (&p, sizeof (void *), 0) failed.");
>>> + free (p);
>> 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.