[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