On 03/11/2016 13:47, Andrew Jones wrote:
> On Thu, Nov 03, 2016 at 01:02:03PM +0100, Laurent Vivier wrote:
>>
>>
>> On 02/11/2016 21:52, Andrew Jones wrote:
>>> This allows us to remove a bunch of memsets.
>>>
>>> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
>>> ---
>>> lib/alloc.c | 8 ++++++++
>>> lib/alloc.h | 1 +
>>> lib/x86/vm.c | 4 +---
>>> x86/vmx.c | 19 +++++++------------
>>> x86/vmx_tests.c | 28 ++++++++++------------------
>>> 5 files changed, 27 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/lib/alloc.c b/lib/alloc.c
>>> index ce1198e2977f..d797690cc458 100644
>>> --- a/lib/alloc.c
>>> +++ b/lib/alloc.c
>>> @@ -191,6 +191,14 @@ void *alloc_page(void)
>>> return p;
>>> }
>>>
>>> +void *alloc_zeroed_page(void)
>>> +{
>>> + void *p = alloc_page();
>>
>> alloc_page() can return NULL.
>>
>> As most of the callers don't check the return value of
>> alloc_zeroed_page(), I think you should assert() here.
>
> Hmm... both returning NULL (which you're right I should have done here)
> and asserting have their merits. If a unit test, for whatever reason,
> wanted to allocate all memory, then it would impossible to know how many
> alloc_page calls that would take before needing to stop to avoid the
> assert. It's much easier to loop until NULL. However, most callers won't
> do that, and, as you point out below, most callers are neglecting to
> check for NULL.
>
> So I think we need both. I propose renaming alloc_page to __alloc_page.
> Unit tests that want to avoid the assert and check for NULL should use
> that. But most callers should use a new alloc_page,
>
> void *alloc_page(void)
> {
> void *p = __alloc_page();
> assert(p);
> return p;
> }
>
> alloc_zeroed_page will be built on alloc_page. __alloc_page users can
> zero their own pages...
>
> Thoughts on that?
It's nice to have both. Good idea.
Thanks,
Laurent
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
