[libvirt] [PATCH v4 5/6] virFileInData: preserve errno in cleanup path

[Michal Privoznik]

On 07/08/2017 02:52 PM, John Ferlan wrote:
> 
> 
> On 06/22/2017 08:30 AM, Michal Privoznik wrote:
>> Callers might be interested in the original value of errno. Let's
>> not overwrite it with lseek() done in cleanup path.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/util/virfile.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
> 
> Looks like I did a lot of writing in my response to the previous
> version.  Still not clear what value this is as it's been pointed out
> previously that lseek shouldn't fail and it had to have succeeded at
> least once (since "cur != -1") and maybe failed at least once (each of
> those getting a ReportSystemError).
> 
> So what value is there in saving errno which may not even be used when
> ret = 0? I don't even see it used when ret = -1. The one thing that does
> happen is saving the errmsg from patch 2, but nothing w/ errno.
> 
> IMO, I think failing that last lseek() should cause failure since the
> API hasn't truthfully represented what it does ("Upon its return, the
> position in the @fd is left unchanged,..."). If that last lseek() fails
> and we return 0, then the caller would be assuming we're back at the
> same position we started, but we're not.  But I think I stated that a
> long time ago when the code was first being added and you convinced me
> otherwise!

Okay, I'll post a patch that does that.

Meanwhile I've pushed the rest modulo this patch. Thanks!

Michal