[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: |
Andrei Borzenkov |
Subject: |
Re: [PATCH V2 1/4] ofnet: add error check for grub_netbuff_reserve |
Date: |
Mon, 12 Dec 2016 15:10:40 +0300 |
On Mon, Dec 12, 2016 at 1:34 PM, Stanislav Kholmanskikh
<address@hidden> wrote:
>
>
> 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?
>
In this place - yes.
> 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.
>
The code is rather inconsistent, I agree, but this is not bug fix that
is required as part of your patch series, so any cleanup belongs
elsewhere.
[PATCH V2 4/4] ofnet: implement the receive buffer, Stanislav Kholmanskikh, 2016/12/02