[libvirt] [PATCH] virFileInData: Preserve errno on error

[Erik Skultety]

On Thu, Oct 18, 2018 at 05:12:33PM +0200, Michal Privoznik wrote:
> The virFileInData() function should return to the caller if the
> current position the passed file is in is a data section or a
> hole (and also how long the current section is). At any rate,
> upon return from this function (be it successful or not) the
> original position in the file is restored. This may mess up with
> errno which might have been set earlier. Save the errno into a
> local variable so it can be restored for the caller's sake.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/util/virfile.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index e09992e41a..f6f01ec1e1 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -4072,11 +4072,18 @@ virFileInData(int fd,
>      ret = 0;
>   cleanup:
>      /* At any rate, reposition back to where we started. */
> -    if (cur != (off_t) -1 &&
> -        lseek(fd, cur, SEEK_SET) == (off_t) -1) {
> -        virReportSystemError(errno, "%s",
> -                             _("unable to restore position in file"));
> -        ret = -1;
> +    if (cur != (off_t) -1) {
> +        int theerrno = errno;

quick grepping through the code shows that we're fairly consistent
(surprisingly) in naming here and call ^this save_errno instead.

> +
> +        if (lseek(fd, cur, SEEK_SET) == (off_t) -1) {
> +            virReportSystemError(errno, "%s",
> +                                 _("unable to restore position in file"));
> +            ret = -1;


> +            if (theerrno == 0)
> +                theerrno = errno;

I'm not entirely sold on this condition, we use the errno produced by this
"re-wind" piece of code only if no other errno has been produced until this
point - feels like we should just throw this one away every time, but that's
just the feeling I get, not that it would matter much.

Reviewed-by: Erik Skultety <eskultet at redhat.com>