|
|
6aa05d |
From 22572f8ac13e2e8daf91d227eac2f384303fb5b4 Mon Sep 17 00:00:00 2001
|
|
|
6aa05d |
From: Eric Blake <eblake@redhat.com>
|
|
|
6aa05d |
Date: Thu, 3 Feb 2022 14:25:57 -0600
|
|
|
6aa05d |
Subject: [PATCH] copy: Pass in dummy variable rather than &errno to callback
|
|
|
6aa05d |
|
|
|
6aa05d |
In several places where asynch handlers manually call the provided
|
|
|
6aa05d |
nbd_completion_callback, the value of errno is indeterminate (for
|
|
|
6aa05d |
example, in file-ops.c:file_asynch_read(), the previous call to
|
|
|
6aa05d |
file_synch_read() already triggered exit() on error, but does not
|
|
|
6aa05d |
guarantee what is left in errno on success). As the callback should
|
|
|
6aa05d |
be paying attention to the value of *error (to be fixed in the next
|
|
|
6aa05d |
patch), we are better off ensuring that we pass in a pointer to a
|
|
|
6aa05d |
known-zero value. Besides, passing in &errno carries a risk that if
|
|
|
6aa05d |
the callback uses any other library function that alters errno prior
|
|
|
6aa05d |
to dereferncing *error, it will no longer see the value we passed in.
|
|
|
6aa05d |
Thus, it is easier to use a dummy variable on the stack than to mess
|
|
|
6aa05d |
around with errno and it's magic macro expansion into a thread-local
|
|
|
6aa05d |
storage location.
|
|
|
6aa05d |
|
|
|
6aa05d |
Note that several callsites then check if the callback returned -1,
|
|
|
6aa05d |
and if so assume that the callback has caused errno to now have a sane
|
|
|
6aa05d |
value to pass on to perror. In theory, the fact that we are no longer
|
|
|
6aa05d |
passing in &errno means that if the callback assigns into *error but
|
|
|
6aa05d |
did not otherwise affect errno (a tenuous assumption, given our
|
|
|
6aa05d |
argument above that we could not even guarantee that the callback does
|
|
|
6aa05d |
not accidentally alter errno prior to reading *error), our perror call
|
|
|
6aa05d |
would no longer reflect the intended error value from the callback.
|
|
|
6aa05d |
But in practice, since the callback never actually returned -1, nor
|
|
|
6aa05d |
even assigned into *error, the call to perror is dead code; although I
|
|
|
6aa05d |
have chosen to defer that additional cleanup to the next patch.
|
|
|
6aa05d |
|
|
|
6aa05d |
Message-Id: <20220203202558.203013-5-eblake@redhat.com>
|
|
|
6aa05d |
Acked-by: Richard W.M. Jones <rjones@redhat.com>
|
|
|
6aa05d |
Acked-by: Nir Soffer <nsoffer@redhat.com>
|
|
|
6aa05d |
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
|
|
|
6aa05d |
(cherry picked from commit 794c8ce06e995ebd282e8f2b9465a06140572112)
|
|
|
6aa05d |
Conflicts:
|
|
|
6aa05d |
copy/file-ops.c - no backport of d5f65e56 ("copy: Do not use trim
|
|
|
6aa05d |
for zeroing"), so asynch_trim needed same treatment
|
|
|
6aa05d |
copy/multi-thread-copying.c - context due to missing refactoring
|
|
|
6aa05d |
copy/null-ops.c - no backport of 0b16205e "copy: Implement "null:"
|
|
|
6aa05d |
destination."
|
|
|
6aa05d |
(cherry picked from commit 26e3dcf80815fe2db320d3046aabc2580c2f7a0d)
|
|
|
6aa05d |
---
|
|
|
6aa05d |
copy/file-ops.c | 22 +++++++++++++---------
|
|
|
6aa05d |
copy/multi-thread-copying.c | 8 +++++---
|
|
|
6aa05d |
2 files changed, 18 insertions(+), 12 deletions(-)
|
|
|
6aa05d |
|
|
|
6aa05d |
diff --git a/copy/file-ops.c b/copy/file-ops.c
|
|
|
6aa05d |
index 086348a..cc312b4 100644
|
|
|
6aa05d |
--- a/copy/file-ops.c
|
|
|
6aa05d |
+++ b/copy/file-ops.c
|
|
|
6aa05d |
@@ -1,5 +1,5 @@
|
|
|
6aa05d |
/* NBD client library in userspace.
|
|
|
6aa05d |
- * Copyright (C) 2020 Red Hat Inc.
|
|
|
6aa05d |
+ * Copyright (C) 2020-2022 Red Hat Inc.
|
|
|
6aa05d |
*
|
|
|
6aa05d |
* This library is free software; you can redistribute it and/or
|
|
|
6aa05d |
* modify it under the terms of the GNU Lesser General Public
|
|
|
6aa05d |
@@ -158,10 +158,11 @@ file_asynch_read (struct rw *rw,
|
|
|
6aa05d |
struct command *command,
|
|
|
6aa05d |
nbd_completion_callback cb)
|
|
|
6aa05d |
{
|
|
|
6aa05d |
+ int dummy = 0;
|
|
|
6aa05d |
+
|
|
|
6aa05d |
file_synch_read (rw, slice_ptr (command->slice),
|
|
|
6aa05d |
command->slice.len, command->offset);
|
|
|
6aa05d |
- errno = 0;
|
|
|
6aa05d |
- if (cb.callback (cb.user_data, &errno) == -1) {
|
|
|
6aa05d |
+ if (cb.callback (cb.user_data, &dummy) == -1) {
|
|
|
6aa05d |
perror (rw->name);
|
|
|
6aa05d |
exit (EXIT_FAILURE);
|
|
|
6aa05d |
}
|
|
|
6aa05d |
@@ -172,10 +173,11 @@ file_asynch_write (struct rw *rw,
|
|
|
6aa05d |
struct command *command,
|
|
|
6aa05d |
nbd_completion_callback cb)
|
|
|
6aa05d |
{
|
|
|
6aa05d |
+ int dummy = 0;
|
|
|
6aa05d |
+
|
|
|
6aa05d |
file_synch_write (rw, slice_ptr (command->slice),
|
|
|
6aa05d |
command->slice.len, command->offset);
|
|
|
6aa05d |
- errno = 0;
|
|
|
6aa05d |
- if (cb.callback (cb.user_data, &errno) == -1) {
|
|
|
6aa05d |
+ if (cb.callback (cb.user_data, &dummy) == -1) {
|
|
|
6aa05d |
perror (rw->name);
|
|
|
6aa05d |
exit (EXIT_FAILURE);
|
|
|
6aa05d |
}
|
|
|
6aa05d |
@@ -185,10 +187,11 @@ static bool
|
|
|
6aa05d |
file_asynch_trim (struct rw *rw, struct command *command,
|
|
|
6aa05d |
nbd_completion_callback cb)
|
|
|
6aa05d |
{
|
|
|
6aa05d |
+ int dummy = 0;
|
|
|
6aa05d |
+
|
|
|
6aa05d |
if (!file_synch_trim (rw, command->offset, command->slice.len))
|
|
|
6aa05d |
return false;
|
|
|
6aa05d |
- errno = 0;
|
|
|
6aa05d |
- if (cb.callback (cb.user_data, &errno) == -1) {
|
|
|
6aa05d |
+ if (cb.callback (cb.user_data, &dummy) == -1) {
|
|
|
6aa05d |
perror (rw->name);
|
|
|
6aa05d |
exit (EXIT_FAILURE);
|
|
|
6aa05d |
}
|
|
|
6aa05d |
@@ -199,10 +202,11 @@ static bool
|
|
|
6aa05d |
file_asynch_zero (struct rw *rw, struct command *command,
|
|
|
6aa05d |
nbd_completion_callback cb)
|
|
|
6aa05d |
{
|
|
|
6aa05d |
+ int dummy = 0;
|
|
|
6aa05d |
+
|
|
|
6aa05d |
if (!file_synch_zero (rw, command->offset, command->slice.len))
|
|
|
6aa05d |
return false;
|
|
|
6aa05d |
- errno = 0;
|
|
|
6aa05d |
- if (cb.callback (cb.user_data, &errno) == -1) {
|
|
|
6aa05d |
+ if (cb.callback (cb.user_data, &dummy) == -1) {
|
|
|
6aa05d |
perror (rw->name);
|
|
|
6aa05d |
exit (EXIT_FAILURE);
|
|
|
6aa05d |
}
|
|
|
6aa05d |
diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
|
|
|
6aa05d |
index a7aaa7d..2593ff7 100644
|
|
|
6aa05d |
--- a/copy/multi-thread-copying.c
|
|
|
6aa05d |
+++ b/copy/multi-thread-copying.c
|
|
|
6aa05d |
@@ -1,5 +1,5 @@
|
|
|
6aa05d |
/* NBD client library in userspace.
|
|
|
6aa05d |
- * Copyright (C) 2020 Red Hat Inc.
|
|
|
6aa05d |
+ * Copyright (C) 2020-2022 Red Hat Inc.
|
|
|
6aa05d |
*
|
|
|
6aa05d |
* This library is free software; you can redistribute it and/or
|
|
|
6aa05d |
* modify it under the terms of the GNU Lesser General Public
|
|
|
6aa05d |
@@ -391,6 +391,7 @@ finished_read (void *vp, int *error)
|
|
|
6aa05d |
bool last_is_hole = false;
|
|
|
6aa05d |
uint64_t i;
|
|
|
6aa05d |
struct command *newcommand;
|
|
|
6aa05d |
+ int dummy = 0;
|
|
|
6aa05d |
|
|
|
6aa05d |
/* Iterate over whole blocks in the command, starting on a block
|
|
|
6aa05d |
* boundary.
|
|
|
6aa05d |
@@ -473,7 +474,7 @@ finished_read (void *vp, int *error)
|
|
|
6aa05d |
/* Free the original command since it has been split into
|
|
|
6aa05d |
* subcommands and the original is no longer needed.
|
|
|
6aa05d |
*/
|
|
|
6aa05d |
- free_command (command, &errno);
|
|
|
6aa05d |
+ free_command (command, &dummy);
|
|
|
6aa05d |
}
|
|
|
6aa05d |
|
|
|
6aa05d |
return 1; /* auto-retires the command */
|
|
|
6aa05d |
@@ -498,6 +499,7 @@ static void
|
|
|
6aa05d |
fill_dst_range_with_zeroes (struct command *command)
|
|
|
6aa05d |
{
|
|
|
6aa05d |
char *data;
|
|
|
6aa05d |
+ int dummy = 0;
|
|
|
6aa05d |
|
|
|
6aa05d |
if (destination_is_zero)
|
|
|
6aa05d |
goto free_and_return;
|
|
|
6aa05d |
@@ -541,7 +543,7 @@ fill_dst_range_with_zeroes (struct command *command)
|
|
|
6aa05d |
free (data);
|
|
|
6aa05d |
|
|
|
6aa05d |
free_and_return:
|
|
|
6aa05d |
- free_command (command, &errno);
|
|
|
6aa05d |
+ free_command (command, &dummy);
|
|
|
6aa05d |
}
|
|
|
6aa05d |
|
|
|
6aa05d |
static int
|
|
|
6aa05d |
--
|
|
|
6aa05d |
2.31.1
|
|
|
6aa05d |
|