Re: [Qemu-devel] [PATCH v19 03/16] block: Introduce op_blockers to BlockDriverState

[Markus Armbruster]

Jeff Cody <address@hidden> writes:

> On Mon, May 19, 2014 at 04:37:52PM +0200, Kevin Wolf wrote:
>> Am 19.05.2014 um 16:10 hat Markus Armbruster geschrieben:
>> > Fam Zheng <address@hidden> writes:
[...]
>> > > diff --git a/block.c b/block.c
>> > > index b749d31..32338ca 100644
>> > > --- a/block.c
>> > > +++ b/block.c
[...]
>> > > @@ -5269,6 +5275,75 @@ void bdrv_unref(BlockDriverState *bs)
>> > >      }
>> > >  }
>> > >  
>> > > +struct BdrvOpBlocker {
>> > > +    Error *reason;
>> > > +    QLIST_ENTRY(BdrvOpBlocker) list;
>> > > +};
>> > > +
>> > > +bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error 
>> > > **errp)
>> > > +{
>> > > +    BdrvOpBlocker *blocker;
>> > > +    assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
>> > 
>> > Space between cast and its operand is unusual.  Please don't respin just
>> > for that.
>> 
>> That was a surprising statement for me. Do you have an idea how to grep
>> for casts? I tried '*)' just in order to find _some_ examples of casts,
>> and there doesn't seem to be a clear winner. But if there is one, it
>> appears to be the version with space.
>> 
>> (I won't reject patches with either style.)
>>
>
> This just searches for pointer casts - it isn't perfect, but pretty
> decent if you just want to get a sampling:
>
> grep -E "\([a-zA-Z0-9_]+[\\*\ ]+\)" * -rHnI

Challenge!

I used

    $ gdb -batch -ex "info types" qemu-system-x86_64 | sed -n 's/^typedef .* 
\([^ ]*\);/\1/p' types | sort -u

to find typedef names, turned them into a regexp, and topped it off with
one matching predefined scalar types.  Let that be T.  I git-grepped for
-E '\((T) *\**\)', and found too many sizeof(T), QLIST_ENTRY(T) and
such, so I filtered out the hits where the last non-space letter before
the (T) is a letter.  This passed visual muster, although it's of course
neither 100% complete nor 100% correct.  Close enough.

This gave me roughly 6000 probable casts.  >75% are not followed by
space.  In block-land (as defined by MAINTAINERS), it's close to 80%.

Three thirds majority may not quite qualify for "clear winner" in
matters of code formatting.

I dislike space between cast and operand because the cast operator has a
high operator precedence.