Libguestfs - Feb 2023 - [libnbd PATCH v3 07/29] lib/utils: add async-signal-safe assert()

On 2/20/23 19:21, Laszlo Ersek wrote:
> On 2/15/23 21:57, Eric Blake wrote:
>> On Wed, Feb 15, 2023 at 03:11:36PM +0100, Laszlo Ersek wrote:
>>> +
>>> +  xwrite (STDERR_FILENO, file, strlen (file));
>>> +  xwrite (STDERR_FILENO, ":", 1);
>>
>> Presumably, if our first best-effort xwrite() fails to produce full
>> output, all later xwrite() will hopefully hit the same error condition
>> so that we aren't producing even more-mangled output where later
>> syscalls succeed despite missing context earlier in the overall
>> output.  If it were something we truly wanted to worry about, the
>> solution would be pre-loading the entire message into a single buffer,
>> then calling xwrite() just once - but that's far more effort for
>> something we don't anticipate hitting in normal usage anyways. 
I'm
>> happy if you ignore this whole paragraph of mine.
> 
> Any single buffer presents the problem of sizing the buffer
> appropriately, which we can't do in this context :)
Actually, we *can* do better:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/writev.html

What we're doing here is the textbook use case for scatter-gather (well,
in this case, gather). It's strange that it has taken me one night to
realize this (it occurred to me before falling asleep last night), given
that I heavily used scatter-gather in other, not-so-old, code. (Namely
the edk2 virtiofs driver.)

I'll attempt to replace xwrite() with xwritev().

Laszlo

On 2/21/23 07:33, Laszlo Ersek wrote:
> On 2/20/23 19:21, Laszlo Ersek wrote:
>> On 2/15/23 21:57, Eric Blake wrote:
>>> On Wed, Feb 15, 2023 at 03:11:36PM +0100, Laszlo Ersek wrote:
> 
>>>> +
>>>> +  xwrite (STDERR_FILENO, file, strlen (file));
>>>> +  xwrite (STDERR_FILENO, ":", 1);
>>>
>>> Presumably, if our first best-effort xwrite() fails to produce full
>>> output, all later xwrite() will hopefully hit the same error
condition
>>> so that we aren't producing even more-mangled output where
later
>>> syscalls succeed despite missing context earlier in the overall
>>> output.  If it were something we truly wanted to worry about, the
>>> solution would be pre-loading the entire message into a single
buffer,
>>> then calling xwrite() just once - but that's far more effort
for
>>> something we don't anticipate hitting in normal usage anyways. 
I'm
>>> happy if you ignore this whole paragraph of mine.
>>
>> Any single buffer presents the problem of sizing the buffer
>> appropriately, which we can't do in this context :)
> 
> Actually, we *can* do better:
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/writev.html
> 
> What we're doing here is the textbook use case for scatter-gather
(well,
> in this case, gather). It's strange that it has taken me one night to
> realize this (it occurred to me before falling asleep last night), given
> that I heavily used scatter-gather in other, not-so-old, code. (Namely
> the edk2 virtiofs driver.)
> 
> I'll attempt to replace xwrite() with xwritev().
No, I won't -- writev() is not required to be async-signal-safe, so it's
not good enough for NBD_INTERNAL_FORK_SAFE_ASSERT().

Laszlo