grub-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 1/3] push/pop errno in initrd read file path


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH 1/3] push/pop errno in initrd read file path
Date: Tue, 15 Mar 2016 17:26:22 +0000



Le Fri, Mar 11, 2016 à 11:01 PM, Andrei Borzenkov <address@hidden> a écrit :
12.03.2016 01:00, Vladimir 'phcoder' Serbinenko пишет:
> On Friday, March 11, 2016, Josef Bacik <address@hidden> wrote:
>
>> On 03/11/2016 02:34 PM, Vladimir 'phcoder' Serbinenko wrote:
>>
>>>
>>>
>>> Le ven. 11 mars 2016 19:13, Josef Bacik <address@hidden
>>> <mailto:address@hidden>> a écrit :
>>>
>>>     On 03/11/2016 12:23 PM, Vladimir 'phcoder' Serbinenko wrote:
>>>      >
>>>      >
>>>      > On Friday, March 11, 2016, Josef Bacik <address@hidden
>>>     <mailto:address@hidden>
>>>      > <mailto:address@hidden <mailto:address@hidden>>> wrote:
>>>      >
>>>      >     If you try to load an initrd from http and it errors out we
>>> will
>>>      >     free the initrd
>>>      >     context but continue on because net_tcp_socket_close() will
>>> reset
>>>      >     the grub_errno
>>>      >     as will grub_initrd_close().  So we'll lose the errno and
>>> return
>>>      >     GRUB_ERR_NONE
>>>      >     instead of the original error.  Add push/pulls to the
>>> appropriate
>>>      >     places so we
>>>      >     don't lose our errno.  Thanks,
>>>      >
>>>      > Close functions shouldn't do this. Can you fix them instead? Also
>>>     please
>>>      > add [2.02] to the subjectwhen appropriate, like in this case.
>>>      >
>>>
>>>     So do we not want close functions to do grub_error() at all?  Seems
>>> like
>>>     there may be some cases where we want to know there was an error
>>> closing
>>>     a tcp socket or the initrd?  Maybe not, just want to make sure before
>>> I
>>>     go make these two functions void.
>>>
>>> How can a failure occur in close routines? What can we do with the
>>> failure anyway?
>>>

It is not about problems in close routines themselves, but we call them
to cleanup and lose errors that caused us to clean up.

Yes, and if we don't put the logic to preserve errors in the close functions, we'll have to repeat this code at every call site which is error-prone and is code duplication. 
>>
>> So sending the FIN packet for the tcp close was failing for example.  I
>> don't think we can do anything really, I just don't like doing a patch 4
>> times, so I want to make sure turning these close functions into void's is
>> ok.  Thanks,
>>
>> I prefer void close functions.  Give it another day and if nobody objects
> by then, then it's fine for everyone.
>

May be I miss something obvious here - which close functions do you
mean? grub_net_tcp_close() is void already, like is grub_initrd_close.
So what exactly do you suggest to change?
I suggest to make them preserve grub_errno properly. 

The problem is that we lose error indication from upper level protocols
or preceding code every time we close file or socket due to previous error.
Yes, and we need to change close routines, to preserve grub_errno 

_______________________________________________
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]