On 4/19/23 20:21, Eric Blake wrote: > On Wed, Apr 19, 2023 at 05:31:27PM +0200, Laszlo Ersek wrote: >> Embedding a shell script in a multi-line C string literal is an exercise >> in pain. I can see why the original author (whom I shall not look up with >> git-blame :) ) went for the easy route. Still, we want the source code to >> fit in 80 columns. >> >> At Eric's suggestion, also: >> >> - replace the non-portable control operator "|&" with the portable >> redirection operator "2>&1" and the portable control operator "|"; >> >> - replace the "if ...; then exit 1; else exit 0; fi" constructs with "! >> ...". > > Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks! > >> Notes: >> v2: >> >> - v1: >> https://listman.redhat.com/archives/libguestfs/2023-April/031298.html >> >> - incorporate Eric's recommendations (commit message, code) >> >> tests/requires.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tests/requires.c b/tests/requires.c >> index 199e30605473..2b7031532fe2 100644 >> --- a/tests/requires.c >> +++ b/tests/requires.c >> @@ -57,7 +57,8 @@ requires_qemu_nbd_tls_support (const char *qemu_nbd) >> * interested in the error message that it prints. >> */ >> snprintf (cmd, sizeof cmd, >> - "if %s --object tls-creds-x509,id=tls0 |& grep -sq 'TLS >> credentials support requires GNUTLS'; then exit 1; else exit 0; fi", >> + "! %s --object tls-creds-x509,id=tls0 2>&1 \\\n" >> + " | grep -sq 'TLS credentials support requires GNUTLS'\n", > > The four bytes " \\\n " portion between the two lines are not > essential (the shell grammar doesn't care if there is a newline at > this point); Indeed, but I wanted the shell script's line breaks to follow the C source's line breaks. :) > in fact, if you put the | before \n instead of after, you > can use a newline without needing the \. That's right, but personally I prefer cmd1 \ | cmd2 \ | cmd3 \ | cmd4 over cmd1 | cmd2 | cmd3 | cmd4 because in the latter (especially if the commands are long / complex), it's hard to see where a pipeline ends and a brand new pipeline starts. Placing the pipe symbol at the front gives more visual cues. (In C, things are different: whenever we need expressions spanning multiple lines, we can easily keep the operators at the end of the line. That's because we already have parens in place, e.g. because the expression is the controlling expression of a statement, or because we can simply add parens for the sake of indentation. But I've never seen something like this, in shell: { cmd1 | cmd2 | cmd3 | cmd4; } and anyway I think it would run the risk of the exact same mistake -- forgetting a pipe symbol somewhere would not cause a "compilation error" but runtime misbehavior.) > But I don't see any point > changing this code yet again just to golf out a few more bytes. Let's > leave it as written in your v2. Thanks! :) At least this time all of it was fully intended on my part. Laszlo _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
