On 5/24/23 17:00, Eric Blake wrote:
> While reviewing the previous whitespace-only patch, I noticed several
> casts that are semantically unnecessary in C:
>
> - converting pointers to and from void* does not need a cast except
> for function pointers or when also casting away const (different
> from C++ where casts to or from void* are required for type safety);
> includes cases like python PyCapsule code which uses void*
>
> - casting away const on a string literal (in C, string literals are
> typed 'char *', different from C++ where they are 'const char *';
> that said, C says it is still undefined behavior to modify their
> contents, so string literals should generally be only assigned to
> 'const char *' variables or passed to functions that promise not to
> modify contents); includes cases like ocaml's struct
> custom_operations using 'const char *' as its member type (where
> casting away const just to re-add it is pointless)
>
> - casts used to document what would otherwise be implicit type
> conversions when assigning to integers of a different type or
> passing function parameters not through varargs. While this can be
> useful for documentation purposes, it can also mask bugs when the
> cast uses the wrong intermediate type only to be implicitly
> converted back to the destination type, and it is often easier to
> use optional compiler warning flags to catch instances of unintended
> truncation or sign extension by implicit conversions.
>
> Minimizing semantically unneeded casts lets the remaining casts stand
> out for why they are necessary: casting aways const, casts between
> pointer aliases (such as struct sockaddr*, char* vs unsigned char*,
> ...), and scalars passed through vargs in printf-like functions.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> generator/Python.ml | 2 +-
> generator/states-connect-socket-activation.c | 2 +-
> generator/states-issue-command.c | 2 +-
> lib/utils.c | 4 ++--
> python/handle.c | 2 +-
> ocaml/nbd-c.h | 6 +++---
> tests/opt-set-meta.c | 2 +-
> copy/main.c | 6 +++---
> fuse/nbdfuse.c | 4 ++--
> fuse/operations.c | 2 +-
> ublk/nbdublk.c | 4 ++--
> 11 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/generator/Python.ml b/generator/Python.ml
> index b73f9782..c81878de 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -49,7 +49,7 @@ let
> {
> assert (obj);
> assert (obj != Py_None);
> - return (struct nbd_handle *)PyCapsule_GetPointer(obj, \"nbd_handle\");
> + return PyCapsule_GetPointer(obj, \"nbd_handle\");
> }
>
> /* nbd.Error exception. */
> diff --git a/generator/states-connect-socket-activation.c
> b/generator/states-connect-socket-activation.c
> index 98a39c0e..40a3528c 100644
> --- a/generator/states-connect-socket-activation.c
> +++ b/generator/states-connect-socket-activation.c
> @@ -294,7 +294,7 @@ CONNECT_SA.START:
>
> char buf[32];
> const char *v =
> - nbd_internal_fork_safe_itoa ((long)getpid (), buf, sizeof buf);
> + nbd_internal_fork_safe_itoa (getpid (), buf, sizeof buf);
> NBD_INTERNAL_FORK_SAFE_ASSERT (strlen (v) <=
> sact_var[pid_ofs].value_len);
> strcpy (env.ptr[pid_ofs] + sact_var[pid_ofs].prefix_len, v);
>
> diff --git a/generator/states-issue-command.c
> b/generator/states-issue-command.c
> index 34ef4652..111e131c 100644
> --- a/generator/states-issue-command.c
> +++ b/generator/states-issue-command.c
> @@ -46,7 +46,7 @@ ISSUE_COMMAND.START:
> h->request.type = htobe16 (cmd->type);
> h->request.handle = htobe64 (cmd->cookie);
> h->request.offset = htobe64 (cmd->offset);
> - h->request.count = htobe32 ((uint32_t)cmd->count);
> + h->request.count = htobe32 (cmd->count);
> h->chunks_sent++;
> h->wbuf = &h->request;
> h->wlen = sizeof (h->request);
> diff --git a/lib/utils.c b/lib/utils.c
> index e3e13cdd..5df5ae51 100644
> --- a/lib/utils.c
> +++ b/lib/utils.c
> @@ -154,7 +154,7 @@ nbd_internal_set_querylist (struct nbd_handle *h, char
> **queries)
> const char *
> nbd_internal_fork_safe_itoa (long v, char *buf, size_t bufsize)
> {
> - unsigned long uv = (unsigned long)v;
> + unsigned long uv = v;
> size_t i = bufsize - 1;
> bool neg = false;
>
> @@ -282,7 +282,7 @@ nbd_internal_fork_safe_perror (const char *s)
> #endif
> #endif
> if (!m)
> - m = nbd_internal_fork_safe_itoa ((long)errno, buf, sizeof buf);
> + m = nbd_internal_fork_safe_itoa (errno, buf, sizeof buf);
> xwritel (STDERR_FILENO, s, ": ", m, "\n", (char *)NULL);
>
> /* Restore original errno in case it was disturbed by the system
> diff --git a/python/handle.c b/python/handle.c
> index 8ff6ba81..548f2dfe 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -46,7 +46,7 @@ static inline PyObject *
> put_handle (struct nbd_handle *h)
> {
> assert (h);
> - return PyCapsule_New ((void *)h, "nbd_handle", NULL);
> + return PyCapsule_New (h, "nbd_handle", NULL);
> }
>
> PyObject *
> diff --git a/ocaml/nbd-c.h b/ocaml/nbd-c.h
> index 0cbe36d1..e3abb912 100644
> --- a/ocaml/nbd-c.h
> +++ b/ocaml/nbd-c.h
> @@ -49,7 +49,7 @@ static inline value
> caml_alloc_initialized_string (mlsize_t len, const char *p)
> {
> value sv = caml_alloc_string (len);
> - memcpy ((char *)String_val (sv), p, len);
> + memcpy (String_val (sv), p, len);
> return sv;
> }
> #endif
> @@ -70,7 +70,7 @@ extern void nbd_internal_ocaml_exception_in_wrapper (const
> char *, value);
> #define NBD_val(v) (*((struct nbd_handle **)Data_custom_val (v)))
>
> static struct custom_operations libnbd_custom_operations = {
> - (char *)"libnbd_custom_operations",
> + "libnbd_custom_operations",
> nbd_internal_ocaml_handle_finalize,
> custom_compare_default,
> custom_hash_default,
> @@ -110,7 +110,7 @@ struct nbd_buffer {
> #define NBD_buffer_val(v) ((struct nbd_buffer *)Data_custom_val (v))
>
> static struct custom_operations nbd_buffer_custom_operations = {
> - (char *)"nbd_buffer_custom_operations",
> + "nbd_buffer_custom_operations",
> nbd_internal_ocaml_buffer_finalize,
> custom_compare_default,
> custom_hash_default,
> diff --git a/tests/opt-set-meta.c b/tests/opt-set-meta.c
> index 1bd60a9c..06f503af 100644
> --- a/tests/opt-set-meta.c
> +++ b/tests/opt-set-meta.c
> @@ -219,7 +219,7 @@ main (int argc, char *argv[])
> * or newer with its --no-sr kill switch.
> */
> requires ("nbdkit --no-sr --help");
> - args[ARRAY_SIZE (args) - 2] = (char *)"--no-sr";
> + args[ARRAY_SIZE (args) - 2] = "--no-sr";
> nbd = nbd_create ();
> if (nbd == NULL ||
> nbd_set_opt_mode (nbd, true) == -1 ||
> diff --git a/copy/main.c b/copy/main.c
> index 391c0c4f..9449440e 100644
> --- a/copy/main.c
> +++ b/copy/main.c
> @@ -371,7 +371,7 @@ main (int argc, char *argv[])
> #else
> t = 1;
> #endif
> - threads = (unsigned)t;
> + threads = t;
> }
>
> if (synchronous)
> @@ -534,7 +534,7 @@ open_local (const char *filename, direction d)
> }
> if (S_ISREG (stat.st_mode)) /* Regular file. */
> return file_create (filename, fd,
> - stat.st_size, (uint64_t)stat.st_blksize, false, d);
> + stat.st_size, stat.st_blksize, false, d);
> else if (S_ISBLK (stat.st_mode)) { /* Block device. */
> unsigned int blkioopt;
>
> @@ -549,7 +549,7 @@ open_local (const char *filename, direction d)
> #endif
>
> return file_create (filename, fd,
> - stat.st_size, (uint64_t)blkioopt, true, d);
> + stat.st_size, blkioopt, true, d);
> }
> else { /* Probably stdin/stdout, a pipe or a socket. */
> synchronous = true; /* Force synchronous mode for pipes. */
> diff --git a/fuse/nbdfuse.c b/fuse/nbdfuse.c
> index 491f6db8..35d5ffac 100644
> --- a/fuse/nbdfuse.c
> +++ b/fuse/nbdfuse.c
> @@ -415,7 +415,7 @@ main (int argc, char *argv[])
> handles_append (&nbd, h); /* reserved above, so can't fail */
> }
> }
> - connections = (unsigned)nbd.len;
> + connections = nbd.len;
> if (verbose)
> fprintf (stderr, "nbdfuse: connections=%u\n", connections);
>
> @@ -424,7 +424,7 @@ main (int argc, char *argv[])
> fprintf (stderr, "%s\n", nbd_get_error ());
> exit (EXIT_FAILURE);
> }
> - size = (uint64_t)ssize;
> + size = ssize;
>
> /* If the remote NBD server is readonly, then act as if the '-r'
> * flag was given on the nbdfuse command line.
> diff --git a/fuse/operations.c b/fuse/operations.c
> index 2d0634dc..e487d150 100644
> --- a/fuse/operations.c
> +++ b/fuse/operations.c
> @@ -396,7 +396,7 @@ nbdfuse_read (const char *path, char *buf,
>
> CHECK_NBD_ASYNC_ERROR (nbd_aio_pread (h, buf, count, offset, cb, 0));
>
> - return (int)count;
> + return count;
> }
>
> static int
> diff --git a/ublk/nbdublk.c b/ublk/nbdublk.c
> index b85ac609..2f097f3e 100644
> --- a/ublk/nbdublk.c
> +++ b/ublk/nbdublk.c
> @@ -347,7 +347,7 @@ main (int argc, char *argv[])
> handles_append (&nbd, h); /* reserved above, so can't fail */
> }
> }
> - connections = (unsigned)nbd.len;
> + connections = nbd.len;
>
> /* Get the size and preferred block sizes. */
> rs = nbd_get_size (nbd.ptr[0]);
> @@ -355,7 +355,7 @@ main (int argc, char *argv[])
> fprintf (stderr, "%s\n", nbd_get_error ());
> exit (EXIT_FAILURE);
> }
> - size = (uint64_t)rs;
> + size = rs;
>
> rs = nbd_get_block_size (nbd.ptr[0], LIBNBD_SIZE_MAXIMUM);
> if (rs <= 0 || rs > 64 * 1024 * 1024)
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
