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