Re: [Libguestfs] [libnbd PATCH v2 4/5] 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; 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