Eric Blake
2023-May-10 15:14 UTC
[Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}
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> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Laszlo Ersek
2023-May-11 06:45 UTC
[Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}
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