[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 04/33] block: Add BdrvChildRole to BdrvChild

From: Eric Blake
Subject: Re: [PATCH v2 04/33] block: Add BdrvChildRole to BdrvChild
Date: Tue, 11 Feb 2020 09:37:28 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/6/20 4:53 AM, Max Reitz wrote:

+++ b/block.c
@@ -2381,6 +2381,7 @@ static void bdrv_replace_child(BdrvChild *child,
BlockDriverState *new_bs)
   BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
                                     const char *child_name,
                                     const BdrvChildClass *child_class,
+                                  BdrvChildRole child_role,

Hmm - C is loose enough to allow the declaration of a parameter as an
enum type even when its intended usage is to receive a bitwise-or
derived from that enum but not declared in the enum.  For example, if I
understand intent correctly, a caller might pass in 0x3
BdrvChildRole.  It feels like it might be cleaner to document the
interface as taking an unsigned int (although then you've lost the
documentation that the int is derived from BdrvChildRole), than to abuse
the typesystem to pass values that are not BdrvChildRole through the

I don’t necessarily disagree, but we have pre-existing examples of such
abuse, like BdrvRequestFlags.

The advantage of using BdrvChildRole as a type here is to show that we
expect values from that enum.  I personally prefer that.

Yeah, the self-documenting aspect is nice.

I mean, we could do something else entirely and name the enum
“BdrvChildRoleBits” and add a “typedef unsigned int BdrvChildRole;”.  I
don’t think we’ve ever done that before but maybe it’d be the cleanest way?

You're right that we haven't done it, but it is also the slickest solution that preserves documentation intent. In C, such a typedef serves only as documentation (and the compiler doesn't enforce it, compared to what a strongly-typed language would do), but I do like the idea.

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]