grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V2 1/4] ofnet: add error check for grub_netbuff_reserve


From: Stanislav Kholmanskikh
Subject: Re: [PATCH V2 1/4] ofnet: add error check for grub_netbuff_reserve
Date: Mon, 12 Dec 2016 13:34:14 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0


On 12/10/2016 09:08 PM, Andrei Borzenkov wrote:
> 02.12.2016 18:10, Stanislav Kholmanskikh пишет:
>> Signed-off-by: Stanislav Kholmanskikh <address@hidden>
>> ---
>>  grub-core/net/drivers/ieee1275/ofnet.c |    6 +++++-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/grub-core/net/drivers/ieee1275/ofnet.c 
>> b/grub-core/net/drivers/ieee1275/ofnet.c
>> index 6bd3b92..8332d34 100644
>> --- a/grub-core/net/drivers/ieee1275/ofnet.c
>> +++ b/grub-core/net/drivers/ieee1275/ofnet.c
>> @@ -90,7 +90,11 @@ get_card_packet (struct grub_net_card *dev)
>>      return NULL;
>>    /* Reserve 2 bytes so that 2 + 14/18 bytes of ethernet header is divisible
>>       by 4. So that IP header is aligned on 4 bytes. */
>> -  grub_netbuff_reserve (nb, 2);
>> +  if (grub_netbuff_reserve (nb, 2))
>> +    {
>> +      grub_netbuff_free (nb);
>> +      return NULL;
>> +    }
>>  
>>    start_time = grub_get_time_ms ();
>>    do
>>
> 
> We already account for this reserve when we allocate netbuf. So this is
> redundant. May be short comment before grub_netbuf_alloc to explain how
> we get at final size.

So your message is that since nb = grub_netbuff_alloc(some_len + 2)
succeeds (i.e. returns a valid buffer), then following
grub_netbuff_reserve(nb, 2) will never fail and needs no check for
error. Right?

Personally I don't like skipping such checks, but if you insist on
removing the check, then, I think, all other drivers should be updated
as well, because they all have the check.

Thanks.

> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]