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

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(a)redhat.com&gt; > --- > 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!