[Libguestfs] [PATCH v4 02/24] nbd: Consistent typedef usage in header

Vladimir Sementsov-Ogievskiy vsementsov at yandex-team.ru
Mon Jun 12 11:59:31 UTC 2023


On 08.06.23 17:17, Eric Blake wrote:
> On Thu, Jun 08, 2023 at 08:56:31AM -0500, Eric Blake wrote:
>> We had a mix of struct declarataions followed by typedefs, and direct
> 
> declarations
> 
>> struct definitions as part of a typedef.  Pick a single style.  Also
>> float a couple of opaque typedefs earlier in the file, as a later
>> patch wants to refer NBDExport* in NBDRequest.  No semantic impact.
> 
> The curse of writing a commit message and then rebasing to a different
> idea; in patch 22, I had originally intended to make NBDMetaContexts a
> concrete type in nbd.h (which depends on NBDExport*, and would be
> directly used in NBDRequest, which in turn is declared before the
> pre-patch mention of NBDExport), but then changed my mind to instead
> have NBDMetaContexts itself also be an opaque type with NBDRequest
> only using NBDMetaContexts*.  And I missed floating the typedef for
> NBDClientConnection to the same point, because we somewhat separated
> opaque types along the lines of which .c files provide various
> functions and opaque types.
> 
>> @@ -26,24 +26,25 @@
>>   #include "qapi/error.h"
>>   #include "qemu/bswap.h"
>>
>> +typedef struct NBDExport NBDExport;
>> +typedef struct NBDClient NBDClient;
>> +
> 
> Preferences on how I should tweak that aspect of this patch?  Options:
> 
> - Don't float NBDExport or NBDClient, and drop that part of the commit
>    message.  However, the later patch that adds the typedef for
>    NBDMetaContexts still has to do it earlier than the definition of
>    NBDRequest, rather than alongside the other opaque types relevant to
>    server.c
> 
> - Also float NBDClientConnection up here, and reword the commit
>    message along the lines of: Also float forward declarations of
>    opaque types to the top of the file, rather than interspersed with
>    function declarations, which will help a future patch that wants to
>    expose yet another opaque type that will be referenced in
>    NBDRequest.

Sounds good to me. Anyway:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov at yandex-team.ru>

> 
> - something else?
> 

-- 
Best regards,
Vladimir



More information about the Libguestfs mailing list