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

[Andrei Borzenkov <arvidjaar@gmail.com>]

12.03.2016 01:00, Vladimir 'phcoder' Serbinenko пишет:
> On Friday, March 11, 2016, Josef Bacik <jbacik@fb.com> wrote:
> 
>> On 03/11/2016 02:34 PM, Vladimir 'phcoder' Serbinenko wrote:
>>
>>>
>>>
>>> Le ven. 11 mars 2016 19:13, Josef Bacik <jbacik@fb.com
>>> <mailto:jbacik@fb.com>> a écrit :
>>>
>>>     On 03/11/2016 12:23 PM, Vladimir 'phcoder' Serbinenko wrote:
>>>      >
>>>      >
>>>      > On Friday, March 11, 2016, Josef Bacik <jbacik@fb.com
>>>     <mailto:jbacik@fb.com>
>>>      > <mailto:jbacik@fb.com <mailto:jbacik@fb.com>>> 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.

>>
>> 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?

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.