[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] 9pfs: Fix some return statements in the synth backend
From: |
Markus Armbruster |
Subject: |
Re: [PATCH] 9pfs: Fix some return statements in the synth backend |
Date: |
Mon, 28 Nov 2022 11:18:48 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Greg Kurz <groug@kaod.org> writes:
> On Mon, 28 Nov 2022 08:35:22 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Greg Kurz <groug@kaod.org> writes:
>>
>> > The qemu_v9fs_synth_mkdir() and qemu_v9fs_synth_add_file() functions
>> > currently return a positive errno value on failure. This causes
>> > checkpatch.pl to spit several errors like the one below:
>> >
>> > ERROR: return of an errno should typically be -ve (return -EAGAIN)
>> > #79: FILE: hw/9pfs/9p-synth.c:79:
>> > + return EAGAIN;
>> >
>> > Simply change the sign. This has no consequence since callers
>> > assert() the returned value to be equal to 0.
>>
>> Out of curiosity: why is assert() appropriate?
>>
>
> Most of the code base comes from the original synth backend which
> was designed to expose QEMU internals to the guest using 9p. The
> hope of the virtio-9p authors was that each QEMU subsystem would
> create its own tree using these two functions (note that they
> are declared extern). Of course these never happened and the synth
> backend remained nearly dead code for years, until finally it got
> re-used to implement 9p qtest. In this context, failure to create a
> synthetic directory or file means the related test has a bug (e.g.
> messing with the paths used by some other test). This code likely
> needs improvements but we never got to it.
I was about to suggest putting this in a file comment, but then I saw
/*
* Not so fast! You might want to read the 9p developer docs first:
* https://wiki.qemu.org/Documentation/9p
*/
and behind the link, there's a paragraph "3. synth fs driver".
Perhaps a brief note on the use of assert() in synth_init() would still
make sense. Up to you.
Thanks!
[...]