* [kvm-unit-tests PATCH] alloc: Add more memalign asserts
@ 2019-11-04 9:20 Janosch Frank
2019-11-04 10:01 ` David Hildenbrand
2019-11-04 10:07 ` Paolo Bonzini
0 siblings, 2 replies; 5+ messages in thread
From: Janosch Frank @ 2019-11-04 9:20 UTC (permalink / raw)
To: kvm; +Cc: thuth, david, pbonzini
Let's test for size and alignment in memalign to catch invalid input
data. Also we need to test for NULL after calling the memalign
function of the registered alloc operations.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
Tested only under s390, tests under other architectures are highly
appreciated.
---
lib/alloc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/alloc.c b/lib/alloc.c
index ecdbbc4..eba9dd6 100644
--- a/lib/alloc.c
+++ b/lib/alloc.c
@@ -46,6 +46,7 @@ void *memalign(size_t alignment, size_t size)
uintptr_t blkalign;
uintptr_t mem;
+ assert(size && alignment);
assert(alloc_ops && alloc_ops->memalign);
if (alignment <= sizeof(uintptr_t))
alignment = sizeof(uintptr_t);
@@ -56,6 +57,8 @@ void *memalign(size_t alignment, size_t size)
size = ALIGN(size + METADATA_EXTRA, alloc_ops->align_min);
p = alloc_ops->memalign(blkalign, size);
+ assert(p != NULL);
+
/* Leave room for metadata before aligning the result. */
mem = (uintptr_t)p + METADATA_EXTRA;
mem = ALIGN(mem, alignment);
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH] alloc: Add more memalign asserts
2019-11-04 9:20 [kvm-unit-tests PATCH] alloc: Add more memalign asserts Janosch Frank
@ 2019-11-04 10:01 ` David Hildenbrand
2019-11-04 10:07 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: David Hildenbrand @ 2019-11-04 10:01 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: thuth, pbonzini
On 04.11.19 10:20, Janosch Frank wrote:
> Let's test for size and alignment in memalign to catch invalid input
> data. Also we need to test for NULL after calling the memalign
> function of the registered alloc operations.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>
> Tested only under s390, tests under other architectures are highly
> appreciated.
>
> ---
> lib/alloc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/alloc.c b/lib/alloc.c
> index ecdbbc4..eba9dd6 100644
> --- a/lib/alloc.c
> +++ b/lib/alloc.c
> @@ -46,6 +46,7 @@ void *memalign(size_t alignment, size_t size)
> uintptr_t blkalign;
> uintptr_t mem;
>
> + assert(size && alignment);
> assert(alloc_ops && alloc_ops->memalign);
> if (alignment <= sizeof(uintptr_t))
> alignment = sizeof(uintptr_t);
> @@ -56,6 +57,8 @@ void *memalign(size_t alignment, size_t size)
> size = ALIGN(size + METADATA_EXTRA, alloc_ops->align_min);
> p = alloc_ops->memalign(blkalign, size);
>
> + assert(p != NULL);
> +
> /* Leave room for metadata before aligning the result. */
> mem = (uintptr_t)p + METADATA_EXTRA;
> mem = ALIGN(mem, alignment);
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH] alloc: Add more memalign asserts
2019-11-04 9:20 [kvm-unit-tests PATCH] alloc: Add more memalign asserts Janosch Frank
2019-11-04 10:01 ` David Hildenbrand
@ 2019-11-04 10:07 ` Paolo Bonzini
2019-11-04 10:12 ` Janosch Frank
1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2019-11-04 10:07 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: thuth, david
On 04/11/19 10:20, Janosch Frank wrote:
> Let's test for size and alignment in memalign to catch invalid input
> data. Also we need to test for NULL after calling the memalign
> function of the registered alloc operations.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>
> Tested only under s390, tests under other architectures are highly
> appreciated.
>
> ---
> lib/alloc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/alloc.c b/lib/alloc.c
> index ecdbbc4..eba9dd6 100644
> --- a/lib/alloc.c
> +++ b/lib/alloc.c
> @@ -46,6 +46,7 @@ void *memalign(size_t alignment, size_t size)
> uintptr_t blkalign;
> uintptr_t mem;
>
> + assert(size && alignment);
Do we want to return NULL instead on !size? This is how malloc(3) is
documented.
Paolo
> assert(alloc_ops && alloc_ops->memalign);
> if (alignment <= sizeof(uintptr_t))
> alignment = sizeof(uintptr_t);
> @@ -56,6 +57,8 @@ void *memalign(size_t alignment, size_t size)
> size = ALIGN(size + METADATA_EXTRA, alloc_ops->align_min);
> p = alloc_ops->memalign(blkalign, size);
>
> + assert(p != NULL);
> +
> /* Leave room for metadata before aligning the result. */
> mem = (uintptr_t)p + METADATA_EXTRA;
> mem = ALIGN(mem, alignment);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH] alloc: Add more memalign asserts
2019-11-04 10:07 ` Paolo Bonzini
@ 2019-11-04 10:12 ` Janosch Frank
2019-11-04 10:16 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Janosch Frank @ 2019-11-04 10:12 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: thuth, david
[-- Attachment #1.1: Type: text/plain, Size: 1556 bytes --]
On 11/4/19 11:07 AM, Paolo Bonzini wrote:
> On 04/11/19 10:20, Janosch Frank wrote:
>> Let's test for size and alignment in memalign to catch invalid input
>> data. Also we need to test for NULL after calling the memalign
>> function of the registered alloc operations.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>
>> Tested only under s390, tests under other architectures are highly
>> appreciated.
>>
>> ---
>> lib/alloc.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/alloc.c b/lib/alloc.c
>> index ecdbbc4..eba9dd6 100644
>> --- a/lib/alloc.c
>> +++ b/lib/alloc.c
>> @@ -46,6 +46,7 @@ void *memalign(size_t alignment, size_t size)
>> uintptr_t blkalign;
>> uintptr_t mem;
>>
>> + assert(size && alignment);
>
> Do we want to return NULL instead on !size? This is how malloc(3) is
> documented.
>
> Paolo
I myself never check for NULL on a unit test and therefore added the
asserts to have it fail visibly.
But sure, we can return NULL for both asserts.
>
>> assert(alloc_ops && alloc_ops->memalign);
>> if (alignment <= sizeof(uintptr_t))
>> alignment = sizeof(uintptr_t);
>> @@ -56,6 +57,8 @@ void *memalign(size_t alignment, size_t size)
>> size = ALIGN(size + METADATA_EXTRA, alloc_ops->align_min);
>> p = alloc_ops->memalign(blkalign, size);
>>
>> + assert(p != NULL);
>> +
>> /* Leave room for metadata before aligning the result. */
>> mem = (uintptr_t)p + METADATA_EXTRA;
>> mem = ALIGN(mem, alignment);
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [kvm-unit-tests PATCH] alloc: Add more memalign asserts
2019-11-04 10:12 ` Janosch Frank
@ 2019-11-04 10:16 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-11-04 10:16 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: thuth, david
[-- Attachment #1.1: Type: text/plain, Size: 1798 bytes --]
On 04/11/19 11:12, Janosch Frank wrote:
> On 11/4/19 11:07 AM, Paolo Bonzini wrote:
>> On 04/11/19 10:20, Janosch Frank wrote:
>>> Let's test for size and alignment in memalign to catch invalid input
>>> data. Also we need to test for NULL after calling the memalign
>>> function of the registered alloc operations.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>
>>> Tested only under s390, tests under other architectures are highly
>>> appreciated.
>>>
>>> ---
>>> lib/alloc.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/lib/alloc.c b/lib/alloc.c
>>> index ecdbbc4..eba9dd6 100644
>>> --- a/lib/alloc.c
>>> +++ b/lib/alloc.c
>>> @@ -46,6 +46,7 @@ void *memalign(size_t alignment, size_t size)
>>> uintptr_t blkalign;
>>> uintptr_t mem;
>>>
>>> + assert(size && alignment);
>>
>> Do we want to return NULL instead on !size? This is how malloc(3) is
>> documented.
>>
>> Paolo
>
> I myself never check for NULL on a unit test and therefore added the
> asserts to have it fail visibly.
>
> But sure, we can return NULL for both asserts.
Hmm yeah for ->memalign it makes sense since unit tests won't SIGSEGV.
For !size let's return NULL, it can simplify code a bit.
Paolo
>>
>>> assert(alloc_ops && alloc_ops->memalign);
>>> if (alignment <= sizeof(uintptr_t))
>>> alignment = sizeof(uintptr_t);
>>> @@ -56,6 +57,8 @@ void *memalign(size_t alignment, size_t size)
>>> size = ALIGN(size + METADATA_EXTRA, alloc_ops->align_min);
>>> p = alloc_ops->memalign(blkalign, size);
>>>
>>> + assert(p != NULL);
>>> +
>>> /* Leave room for metadata before aligning the result. */
>>> mem = (uintptr_t)p + METADATA_EXTRA;
>>> mem = ALIGN(mem, alignment);
>>>
>>
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-11-04 10:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 9:20 [kvm-unit-tests PATCH] alloc: Add more memalign asserts Janosch Frank
2019-11-04 10:01 ` David Hildenbrand
2019-11-04 10:07 ` Paolo Bonzini
2019-11-04 10:12 ` Janosch Frank
2019-11-04 10:16 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).