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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
