The purpose of this function is to tell if the current position
in given FD is in data section or a hole and how much bytes there
is remaining until the end of the section. This is achieved by
couple of lseeks(). The most important part is that we reposition
the FD back, so that the position is unchanged from the caller
POV. And until now the final lseek() back to the original
position was done with no check for errors. And I was convinced
that that's okay since nothing can go wrong. However, John
persuaded me, that it's better to be safe than sorry. Therefore,
lets check if the final lseek() succeeded and if it doesn't
report an error.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virfile.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index d444b32f8..2f28e83f4 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3900,8 +3900,12 @@ virFileInData(int fd,
     ret = 0;
  cleanup:
     /* At any rate, reposition back to where we started. */
-    if (cur != (off_t) -1)
-        ignore_value(lseek(fd, cur, SEEK_SET));
+    if (cur != (off_t) -1 &&
+        lseek(fd, cur, SEEK_SET) == (off_t) -1) {
+        virReportSystemError(errno, "%s",
+                             _("unable to restore position in file"));
+        ret = -1;
+    }
     return ret;
 }
 
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/11/2017 05:05 AM, Michal Privoznik wrote:
> The purpose of this function is to tell if the current position
> in given FD is in data section or a hole and how much bytes there
> is remaining until the end of the section. This is achieved by
> couple of lseeks(). The most important part is that we reposition
> the FD back, so that the position is unchanged from the caller
> POV. And until now the final lseek() back to the original
> position was done with no check for errors. And I was convinced
> that that's okay since nothing can go wrong. However, John

Good thing there's only one John ;-)   Perhaps better said, "review
feedback from a related series persuaded me..."

> persuaded me, that it's better to be safe than sorry. Therefore,
> lets check if the final lseek() succeeded and if it doesn't
> report an error.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virfile.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/18/2017 11:04 PM, John Ferlan wrote:
> 
> 
> On 07/11/2017 05:05 AM, Michal Privoznik wrote:
>> The purpose of this function is to tell if the current position
>> in given FD is in data section or a hole and how much bytes there
>> is remaining until the end of the section. This is achieved by
>> couple of lseeks(). The most important part is that we reposition
>> the FD back, so that the position is unchanged from the caller
>> POV. And until now the final lseek() back to the original
>> position was done with no check for errors. And I was convinced
>> that that's okay since nothing can go wrong. However, John
> 
> Good thing there's only one John ;-)   Perhaps better said, "review
> feedback from a related series persuaded me..."
> 
>> persuaded me, that it's better to be safe than sorry. Therefore,
>> lets check if the final lseek() succeeded and if it doesn't
>> report an error.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/util/virfile.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> John
> 

Fixed and pushed. Thank you.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list