[Libguestfs] [libnbd PATCH] maint: pick consistent spacing style for casts

[Laszlo Ersek]

On 5/24/23 04:12, Eric Blake wrote:
> We are inconsistent on whether a cast operator should be separated
> from its argument by a space.  As a unary prefix operator, casts bind
> with relatively tight precedence, where only postfix operations
> (including function calls) come higher.  We generally don't use a
> space after other prefix operators (*, &, ~, and unary - tend to not
> be followed by space; we are less consistent on !, but that would be a
> separate patch).
> 
> Furthermore, we've already decided (in commit b5101fbc) that we prefer
> spaces before function invocations.  At the time, discussion on the
> list[1] pointed out that thanks to the complexity of C with its
> context-sensitive parsing needing to know whether an identifier is a
> type name, you would be able to tell the difference between a
> 2-argument function call with two parameters through a function
> pointer (or intentional function-like macro suppression):
> 
>   (foo) (bar, baz)
> 
> from the (admittedly rare) application of a cast operator to the
> results of a comma operator:
> 
>   (foo)(bar, baz)
> 
> if we consistently avoid spaces after casts.
> 
> Determining cast operators is not always trivial, but the regex used
> below seems to have a pretty low false positive rate (lines it selects
> usually are casts and not in a comment line), and can be modified by
> removing the space to see where we already had casts without a
> space. (Obviously, I can't tell how many false negatives there are of
> casts I missed out on).  While this changes more lines than would be
> done by instead always using a space in a cast, it is still manageable
> to do a bulk change.
> 
> $ git grep '\(_t\|int\|long\|signed\|char\|\*\)) [a-zA-Z0-9&"]' -- \
>   '**/*.[hc]' '**/*.ml' | grep -v '^ */\?\*'
> 
> 'git show -w' shows that this change is whitespace only.
> 
> [1] https://listman.redhat.com/archives/libguestfs/2023-February/030771.html
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  configure.ac                                 |  6 ++--
>  common/utils/vector.h                        |  4 +--
>  generator/OCaml.ml                           |  4 +--
>  generator/Python.ml                          |  4 +--
>  generator/states-connect-socket-activation.c |  4 +--
>  generator/states-connect.c                   |  2 +-
>  generator/states-issue-command.c             |  2 +-
>  generator/states-newstyle.c                  |  2 +-
>  generator/states-oldstyle.c                  |  2 +-
>  generator/states-reply-structured.c          | 14 ++++----
>  generator/states-reply.c                     |  2 +-
>  generator/states.c                           |  4 +--
>  lib/crypto.c                                 |  6 ++--
>  lib/rw.c                                     |  2 +-
>  lib/uri.c                                    | 10 +++---
>  lib/utils.c                                  |  4 +--
>  common/utils/test-vector.c                   |  4 +--
>  python/handle.c                              |  4 +--
>  python/utils.c                               |  2 +-
>  ocaml/nbd-c.h                                |  6 ++--
>  tests/aio-connect-port.c                     |  2 +-
>  tests/aio-connect.c                          |  2 +-
>  tests/errors-bad-cookie.c                    |  2 +-
>  tests/errors-client-oversize.c               |  2 +-
>  tests/errors-client-unadvertised-cmd.c       |  2 +-
>  tests/errors-client-unaligned.c              |  2 +-
>  tests/errors-client-unknown-flags.c          |  2 +-
>  tests/errors-client-zerosize.c               |  2 +-
>  tests/errors-connect-null.c                  |  2 +-
>  tests/errors-connect-twice.c                 |  4 +--
>  tests/errors-multiple-disconnects.c          |  2 +-
>  tests/errors-not-negotiating.c               |  2 +-
>  tests/errors-notify-not-blocked.c            |  2 +-
>  tests/errors-pread-structured.c              |  2 +-
>  tests/errors-server-invalid-offset.c         |  2 +-
>  tests/errors-server-oversize.c               |  2 +-
>  tests/errors-server-unadvertised-cmd.c       |  2 +-
>  tests/errors-server-unaligned.c              |  2 +-
>  tests/errors-server-unknown-flags.c          |  2 +-
>  tests/errors-server-zerosize.c               |  2 +-
>  tests/opt-set-meta.c                         |  2 +-
>  tests/opt-starttls.c                         |  4 +--
>  tests/opt-structured-twice.c                 |  2 +-
>  tests/pread-initialize.c                     |  2 +-
>  tests/private-data.c                         |  4 +--
>  tests/server-death.c                         |  2 +-
>  tests/shutdown-flags.c                       |  2 +-
>  examples/glib-main-loop.c                    | 36 ++++++++++----------
>  examples/open-qcow2.c                        |  2 +-
>  interop/list-exports.c                       |  2 +-
>  copy/file-ops.c                              |  6 ++--
>  copy/main.c                                  | 10 +++---
>  copy/nbd-ops.c                               | 36 ++++++++++----------
>  copy/pipe-ops.c                              |  8 ++---
>  dump/dump.c                                  |  2 +-
>  fuse/nbdfuse.c                               |  8 ++---
>  fuse/operations.c                            | 10 +++---
>  ublk/nbdublk.c                               |  4 +--
>  ublk/tgt.c                                   |  4 +--
>  59 files changed, 138 insertions(+), 138 deletions(-)

Going through the changes individually, it seems like we could eliminate
a handful of the casts altogether; for examle (char *)"string literal"
ones. (The C standard effectively says that a (non-wide) string literal
has type "static char[n]", not "static const char[n]".) But that would
be a different patch, plus I can imagine we have those casts in the
first place because gcc complained or whatever. :)

Reviewed-by: Laszlo Ersek <lersek at redhat.com>