[Libguestfs] [PATCH nbdkit 2/4] common/protocol: Remove protostrings.sed, use bash+sed instead.

[Eric Blake]

On 9/24/19 4:07 PM, Richard W.M. Jones wrote:
> Use a simple bash script to generate the protostrings.c functions.
> 
> Remove the extern decls from the nbd-protocol.h file which were used
> previously in the generation of this file.  They have been moved to a
> new internal header called "protostrings.h".
> ---
>  common/protocol/Makefile.am                   |  8 ++-
>  ...tostrings.sed => generate-protostrings.sh} | 56 +++++++++++--------

Wow - git thinks .sed and .sh are similar!  Probably more a case of the
boilerplate copyright being identical, and the rest of the file being
short enough in spite of more-or-less wholesale rewrite.

>  common/protocol/nbd-protocol.h                | 10 ----
>  common/protocol/protostrings.h                | 51 +++++++++++++++++
>  plugins/nbd/nbd-standalone.c                  |  1 +
>  server/protocol-handshake-newstyle.c          |  1 +
>  server/protocol.c                             |  1 +
>  7 files changed, 91 insertions(+), 37 deletions(-)
> 

> +protostrings.c: nbd-protocol.h generate-protostrings.sh Makefile
>  	rm -f $@ $@-t
> -	$(SED) -n -f $(srcdir)/protostrings.sed < $< > $@-t
> +	SED=$(SED) $(srcdir)/generate-protostrings.sh > $@-t

Okay, not just a wholesale rewrite, but keeping a sed invocation while
wrapping in even more logic.

> --- a/common/protocol/protostrings.sed
> +++ b/common/protocol/generate-protostrings.sh
> @@ -1,5 +1,6 @@
> +#!/usr/bin/env bash
>  # nbdkit
> -# Copyright (C) 2018 Red Hat Inc.
> +# Copyright (C) 2018-2019 Red Hat Inc.
>  #
>  # Redistribution and use in source and binary forms, with or without
>  # modification, are permitted provided that the following conditions are
> @@ -29,30 +30,37 @@
>  # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>  # SUCH DAMAGE.
>  
> -# Generate the protostrings.c file from nbd-protocol.h.
> +# The header.
> +cat <<'EOF'
> +/* Generated from nbd-protocol.h by generate-protostrings.sh. */

Worth using:

cat << EOF
/* Generated from nbd-protocol.h by $0. */

?  Or is that too prone to non-reproducible builds if $0 includes a full
path (would $(basename -- $0) fix that)?

>  
> -# Prologue.
> -1i\
> -/* Generated from nbd-protocol.h by protostrings.sed. */\
> -\#include "nbd-protocol.h"\
> +#include "nbd-protocol.h"
>  
> +EOF

We don't have a mention of license/copyright in the generated file
(other than implicitly assuming the same license as in the source files
that generated it).  Is now a good time to spit one out as part of our
prologue?

>  
> -# Match the precise sections of the source file.
> -/^extern const char \*name_of_/,/^$/ {
> +declare -A functions=(
> +    [global_flag]=NBD_FLAG_FIXED_NEWSTYLE
> +    [flag]=NBD_FLAG_HAS_FLAGS
> +    [opt]=NBD_OPT_EXPORT_NAME
> +    [rep]=NBD_REP_ACK
> +    [info]=NBD_INFO_EXPORT
> +    [reply]=NBD_REPLY_FLAG_DONE
> +    [reply_type]=NBD_REPLY_TYPE_NONE
> +    [cmd]=NBD_CMD_READ
> +    [cmd_flag]=NBD_CMD_FLAG_FUA
> +    [error]=NBD_SUCCESS
> +)

Definite bashism.  That's fine, given the #! line.

>  
> -  # Convert extern function prototype into a definition.
> -  s/extern \(const char \*name_of_.*\) (int);/\1 (int fl) {\
> -  switch (fl) {/;
> -
> -  # Convert #define lines into cases.
> -  s/^#define \([_A-Z]*\).*/  case \1: return "\1\";/;
> -
> -  # Append closing brace.
> -  s/^$/  default: return "unknown";\
> -  }\
> -}/;
> -
> -  # Print pattern buffer.
> -  p;
> -
> -}
> +# Generate each 'const char *name_of_nbd_<fn>'
> +for fn in "${!functions[@]}"; do

Is this going to be deterministic, or are we at the mercy of bash's
hashing?  Should we sort the list to guarantee stable output order?

> +    echo 'extern const char *'
> +    echo "name_of_nbd_$fn (int fl)"
> +    echo '{'
> +    echo '  switch (fl) {'
> +    $SED -n "/^#define ${functions[$fn]}/,/^$/p" nbd-protocol.h |
> +        $SED 's/^#define \([_A-Z]*\).*/  case \1:\n    return "\1\";/'
> +    echo '  default: return "unknown";'
> +    echo '  }'
> +    echo '}'
> +    echo
> +done

No sanity checking that $# was 0, but for an internal-only build script,
that probably doesn't matter.  Looks like a sane conversion.


> diff --git a/plugins/nbd/nbd-standalone.c b/plugins/nbd/nbd-standalone.c
> index a6779a4..1789e39 100644
> --- a/plugins/nbd/nbd-standalone.c
> +++ b/plugins/nbd/nbd-standalone.c
> @@ -54,6 +54,7 @@
>  
>  #include <nbdkit-plugin.h>
>  #include "nbd-protocol.h"
> +#include "protostrings.h"

Should we switch from "nbd-protocol.h" to <nbd-protocol.h> at some point
in the series, to reflect our intent of installing it? But doesn't
affect this patch.

ACK

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190924/f3513c1b/attachment.sig>