You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
164 lines
5.9 KiB
164 lines
5.9 KiB
From 22572f8ac13e2e8daf91d227eac2f384303fb5b4 Mon Sep 17 00:00:00 2001
|
|
From: Eric Blake <eblake@redhat.com>
|
|
Date: Thu, 3 Feb 2022 14:25:57 -0600
|
|
Subject: [PATCH] copy: Pass in dummy variable rather than &errno to callback
|
|
|
|
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. Besides, passing in &errno carries a risk that if
|
|
the callback uses any other library function that alters errno prior
|
|
to dereferncing *error, it will no longer see the value we passed in.
|
|
Thus, 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 (a tenuous assumption, given our
|
|
argument above that we could not even guarantee that the callback does
|
|
not accidentally alter errno prior to reading *error), our perror call
|
|
would no longer reflect the intended error value from the callback.
|
|
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.
|
|
|
|
Message-Id: <20220203202558.203013-5-eblake@redhat.com>
|
|
Acked-by: Richard W.M. Jones <rjones@redhat.com>
|
|
Acked-by: Nir Soffer <nsoffer@redhat.com>
|
|
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
|
|
(cherry picked from commit 794c8ce06e995ebd282e8f2b9465a06140572112)
|
|
Conflicts:
|
|
copy/file-ops.c - no backport of d5f65e56 ("copy: Do not use trim
|
|
for zeroing"), so asynch_trim needed same treatment
|
|
copy/multi-thread-copying.c - context due to missing refactoring
|
|
copy/null-ops.c - no backport of 0b16205e "copy: Implement "null:"
|
|
destination."
|
|
(cherry picked from commit 26e3dcf80815fe2db320d3046aabc2580c2f7a0d)
|
|
---
|
|
copy/file-ops.c | 22 +++++++++++++---------
|
|
copy/multi-thread-copying.c | 8 +++++---
|
|
2 files changed, 18 insertions(+), 12 deletions(-)
|
|
|
|
diff --git a/copy/file-ops.c b/copy/file-ops.c
|
|
index 086348a..cc312b4 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
|
|
@@ -158,10 +158,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);
|
|
}
|
|
@@ -172,10 +173,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);
|
|
}
|
|
@@ -185,10 +187,11 @@ static bool
|
|
file_asynch_trim (struct rw *rw, struct command *command,
|
|
nbd_completion_callback cb)
|
|
{
|
|
+ int dummy = 0;
|
|
+
|
|
if (!file_synch_trim (rw, command->offset, command->slice.len))
|
|
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);
|
|
}
|
|
@@ -199,10 +202,11 @@ static bool
|
|
file_asynch_zero (struct rw *rw, struct command *command,
|
|
nbd_completion_callback cb)
|
|
{
|
|
+ int dummy = 0;
|
|
+
|
|
if (!file_synch_zero (rw, command->offset, command->slice.len))
|
|
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 a7aaa7d..2593ff7 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
|
|
@@ -391,6 +391,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.
|
|
@@ -473,7 +474,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 */
|
|
@@ -498,6 +499,7 @@ static void
|
|
fill_dst_range_with_zeroes (struct command *command)
|
|
{
|
|
char *data;
|
|
+ int dummy = 0;
|
|
|
|
if (destination_is_zero)
|
|
goto free_and_return;
|
|
@@ -541,7 +543,7 @@ fill_dst_range_with_zeroes (struct command *command)
|
|
free (data);
|
|
|
|
free_and_return:
|
|
- free_command (command, &errno);
|
|
+ free_command (command, &dummy);
|
|
}
|
|
|
|
static int
|
|
--
|
|
2.31.1
|
|
|