Re: [PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tight

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 29/10/20 18:39, Paolo Bonzini wrote:
>>> When @tight was set to false as it should be, absent @tight defaults
>>> to false.  Wrong, it should default to true.  This is what breaks QMP.
>> When @has_tight...
>
> Ah, I see what you meant here.  Suggested reword:
>
> ---------
> An optional bool member of a QAPI struct can be false, true, or absent.
> The previous commit demonstrated that socket_listen() and
> socket_connect() are broken for absent @tight, and indeed QMP chardev-
> add also defaults absent member @tight to false instead of true.
>
> In C, QAPI members are represented by two fields, has_MEMBER and MEMBER.
> We have:
>
>           has_MEMBER    MEMBER
>     false         true           false
>     true        true     false
>     absent     false  false/ignore
>
> When has_MEMBER is false, MEMBER should be set to false on write, and
> ignored on read.
>
> For QMP, the QAPI visitors handle absent @tight by setting both
> @has_tight and @tight to false.  unix_listen_saddr() and
> unix_connect_saddr() however use @tight only, disregarding @has_tight.
> This is wrong and means that absent @tight defaults to false whereas it
> should default to true.
>
> The same is true for @has_abstract, though @abstract defaults to
> false and therefore has the same behavior for all of QMP, HMP and CLI.
> Fix unix_listen_saddr() and unix_connect_saddr() to check
> @has_abstract/@has_tight, and to default absent @tight to true.
>
> However, this is only half of the story.  HMP chardev-add and CLI
> -chardev so far correctly defaulted @tight to true, but defaults to
> false again with the above fix for HMP and CLI.  In fact, the "tight"
> and "abstract" options now break completely.
>
> Digging deeper, we find that qemu_chr_parse_socket() also ignores
> @has_tight, leaving it false when it sets @tight.  That is also wrong,
> but the two wrongs cancelled out.  Fix qemu_chr_parse_socket() to set
> @has_tight and @has_abstract; writing testcases for HMP and CLI is left
> for another day.
> ---------
>
> Apologies if the last sentence is incorrect. :)

Sold (with the table fixed as per Eric's review)!