[Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}

Laszlo Ersek lersek at redhat.com
Thu May 11 06:45:25 UTC 2023


On 5/10/23 17:14, Eric Blake wrote:
> On Wed, May 10, 2023 at 01:48:11PM +0200, Laszlo Ersek wrote:
>> Wrap those comments in "lib/states.h" and "lib/states.c" that describe the
>> automaton's states.
>>
>> Example changes from "lib/states.h":
>>
>>>    /* CONNECT_TCP.CONNECT: Initial call to connect(2) on a TCP socket */
>>>    STATE_CONNECT_TCP_CONNECT,
>>>
>>> -  /* CONNECT_TCP.CONNECTING: Connecting to the remote server over a TCP socket */
>>> +  /* CONNECT_TCP.CONNECTING: Connecting to the remote server over a TCP socket
>>> +   */
>
> This one looks a bit unusual; I didn't find any instances of this
> style in existing hand-written comments (
>   git grep -B1 '^[:space:]*\*/$' | grep '/\*'
> ).  But I saw the code you had in the previous patch that produced it,
> and don't see any way to force the wrap one word earlier in this
> particular instance without adding even more complexity.  So I'm okay
> with how it ended up.
>
> Reviewed-by: Eric Blake <eblake at redhat.com>
>

Thanks!

This comment folding style (breaking off just the terminating '*/') is
common at least in QEMU, as I recall:

  git grep -h -B1 '^ *\*/$' | grep  -A1 '^ */\*'

... The same command also returns 5 hits in libnbd:

> /* Return true if size is a multiple of align. align must be power of 2.
>  */
> --
> /* Round up i to next multiple of n (n must be a power of 2).
>  */
> --
> /* Round down i to next multiple of n (n must be a power of 2).
>  */
> --
> /* Deliberately disconnect while stranding commands, to check their status.
>  */
> --
> /* Deliberately shutdown while stranding commands, to check their status.
>  */

and 9 hits in nbdkit:

>   /* Read bytes from [offset, offset+count-1] and copy into buf.
>    */
> --
> /* Return true if size is a multiple of align. align must be power of 2.
>  */
> --
> /* Round up i to next multiple of n (n must be a power of 2).
>  */
> --
> /* Round down i to next multiple of n (n must be a power of 2).
>  */
> --
> /* Grab the appropriate error value.
>  */
> --
> /* Reply to NBD_OPT_LIST with the plugin's list of export names.
>  */
> --
> /* Parse a string as a boolean, or return -1 after reporting the error.
>  */
> --
> /* This preserves errno, for convenience.
>  */
> --
>   /* Randomly read parts of the disk to ensure we get the same data.
>    */

Interestingly, your variant of the same grep does not produce any
output, and I don't immediately see why...

Ah, I see now. Character classes such as [:space:] *only* work within
bracket expressions. POSIX writes:

    The character sequences "[.", "[=", and "[:" ( <left-square-bracket>
    followed by a <period>, <equals-sign>, or <colon>) shall be special
    inside a bracket expression and are used to delimit collating
    symbols, equivalence class expressions, and character class
    expressions. These symbols shall be followed by a valid expression
    and the matching terminating sequence ".]", "=]", or ":]", as
    described in the following items.

In other words, the sequence "[:space:]" in itself is just a (weird)
bracket expression, matching the 's', 'p', 'a', 'c', 'e', and ':'
(twice) characters.

If we want [:space:] to function as a character class, it must be
embedded inside a bracket expression. So the following works:

  git grep -B1 '^[[:space:]]*\*/$' | grep '/\*'

(And then this form allows for other expressions alongside the character
class within the bracket expression -- for example, *further* character
classes.)

In fact this is the first time I've myself ever understood how character
classes are supposed to be used in grep :/

Laszlo


More information about the Libguestfs mailing list