[Libguestfs] [libnbd PATCH v2 4/5] copy: Pass in dummy variable rather than &errno to callback

Eric Blake eblake at redhat.com
Thu Feb 3 20:25:57 UTC 2022


In several places where asynch handlers manually call the provided
nbd_completion_callback, the value of errno is indeterminate (for
example, in file-ops.c:file_asynch_read(), the previous call to
file_synch_read() already triggered exit() on error, but does not
guarantee what is left in errno on success).  As the callback should
be paying attention to the value of *error (to be fixed in the next
patch), we are better off ensuring that we pass in a pointer to a
known-zero value; at which point, it is easier to use a dummy variable
on the stack than to mess around with errno and it's magic macro
expansion into a thread-local storage location.

Note that several callsites then check if the callback returned -1,
and if so assume that the callback has caused errno to now have a sane
value to pass on to perror.  In theory, the fact that we are no longer
passing in &errno means that if the callback assigns into *error but
did not otherwise affect errno, our perror call would no longer
reflect that value.  But in practice, since the callback never
actually returned -1, nor even assigned into *error, the call to
perror is dead code; although I have chosen to defer that additional
cleanup to the next patch.
---
 copy/file-ops.c             | 17 ++++++++++-------
 copy/multi-thread-copying.c |  8 +++++---
 copy/null-ops.c             | 12 +++++++-----
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/copy/file-ops.c b/copy/file-ops.c
index 84704341..e37a5014 100644
--- a/copy/file-ops.c
+++ b/copy/file-ops.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace.
- * Copyright (C) 2020 Red Hat Inc.
+ * Copyright (C) 2020-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -587,10 +587,11 @@ file_asynch_read (struct rw *rw,
                   struct command *command,
                   nbd_completion_callback cb)
 {
+  int dummy = 0;
+
   file_synch_read (rw, slice_ptr (command->slice),
                    command->slice.len, command->offset);
-  errno = 0;
-  if (cb.callback (cb.user_data, &errno) == -1) {
+  if (cb.callback (cb.user_data, &dummy) == -1) {
     perror (rw->name);
     exit (EXIT_FAILURE);
   }
@@ -601,10 +602,11 @@ file_asynch_write (struct rw *rw,
                    struct command *command,
                    nbd_completion_callback cb)
 {
+  int dummy = 0;
+
   file_synch_write (rw, slice_ptr (command->slice),
                     command->slice.len, command->offset);
-  errno = 0;
-  if (cb.callback (cb.user_data, &errno) == -1) {
+  if (cb.callback (cb.user_data, &dummy) == -1) {
     perror (rw->name);
     exit (EXIT_FAILURE);
   }
@@ -614,10 +616,11 @@ static bool
 file_asynch_zero (struct rw *rw, struct command *command,
                   nbd_completion_callback cb, bool allocate)
 {
+  int dummy = 0;
+
   if (!file_synch_zero (rw, command->offset, command->slice.len, allocate))
     return false;
-  errno = 0;
-  if (cb.callback (cb.user_data, &errno) == -1) {
+  if (cb.callback (cb.user_data, &dummy) == -1) {
     perror (rw->name);
     exit (EXIT_FAILURE);
   }
diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
index b17ca598..815b8a02 100644
--- a/copy/multi-thread-copying.c
+++ b/copy/multi-thread-copying.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace.
- * Copyright (C) 2020 Red Hat Inc.
+ * Copyright (C) 2020-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -393,6 +393,7 @@ finished_read (void *vp, int *error)
     bool last_is_hole = false;
     uint64_t i;
     struct command *newcommand;
+    int dummy = 0;

     /* Iterate over whole blocks in the command, starting on a block
      * boundary.
@@ -475,7 +476,7 @@ finished_read (void *vp, int *error)
     /* Free the original command since it has been split into
      * subcommands and the original is no longer needed.
      */
-    free_command (command, &errno);
+    free_command (command, &dummy);
   }

   return 1; /* auto-retires the command */
@@ -502,6 +503,7 @@ fill_dst_range_with_zeroes (struct command *command)
 {
   char *data;
   size_t data_size;
+  int dummy = 0;

   if (destination_is_zero)
     goto free_and_return;
@@ -537,7 +539,7 @@ fill_dst_range_with_zeroes (struct command *command)
   free (data);

  free_and_return:
-  free_command (command, &errno);
+  free_command (command, &dummy);
 }

 static int
diff --git a/copy/null-ops.c b/copy/null-ops.c
index a38666d6..11cd5116 100644
--- a/copy/null-ops.c
+++ b/copy/null-ops.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace.
- * Copyright (C) 2020-2021 Red Hat Inc.
+ * Copyright (C) 2020-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -117,8 +117,9 @@ null_asynch_write (struct rw *rw,
                    struct command *command,
                    nbd_completion_callback cb)
 {
-  errno = 0;
-  if (cb.callback (cb.user_data, &errno) == -1) {
+  int dummy = 0;
+
+  if (cb.callback (cb.user_data, &dummy) == -1) {
     perror (rw->name);
     exit (EXIT_FAILURE);
   }
@@ -128,8 +129,9 @@ static bool
 null_asynch_zero (struct rw *rw, struct command *command,
                   nbd_completion_callback cb, bool allocate)
 {
-  errno = 0;
-  if (cb.callback (cb.user_data, &errno) == -1) {
+  int dummy = 0;
+
+  if (cb.callback (cb.user_data, &dummy) == -1) {
     perror (rw->name);
     exit (EXIT_FAILURE);
   }
-- 
2.34.1




More information about the Libguestfs mailing list