[Libguestfs] [libnbd PATCH] maint: Drop useless casts

Laszlo Ersek lersek at redhat.com
Wed May 24 16:57:31 UTC 2023


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 <eblake at 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 <lersek at redhat.com>



More information about the Libguestfs mailing list