On Mon, 16 Dec 2024 at 16:14, Peter Xu <pet...@redhat.com> wrote:
>
> QEMU uses g_mkdir_with_parents() a lot, especially in the case where the
> failure case is ignored so an abort is expected when happened.
>
> Provide a helper qemu_mkdir_with_parents() to do that, and use it in the
> two cases in qga/.  To be used in more places later.
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
>  include/qemu/osdep.h     | 7 +++++++
>  qga/commands-posix-ssh.c | 8 ++------
>  util/osdep.c             | 6 ++++++
>  3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index fdff07fd99..dc67fb2e5e 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -828,6 +828,13 @@ static inline int platform_does_not_support_system(const 
> char *command)
>  }
>  #endif /* !HAVE_SYSTEM_FUNCTION */
>
> +/**
> + * qemu_mkdir_with_parents:
> + *
> + * Create directories with parents.  Abort on failures.
> + */
> +void qemu_mkdir_with_parents(const char *dir, int mode);

Don't put new function prototypes into osdep.h, please.
It is included by every single C file in the codebase.
There is always somewhere better to put things.

QEMU shouldn't abort on things that are kind of expected
OS errors like "couldn't create a directory", so I'm
a bit dubious about this function.

The two use cases in this commit seem to be test code,
so asserting is reasonable. But a "for test code only"
function should go in a header file that's only included
by test cases and the comment should be clear about that,
and it shouldn't have a function name that implies
"this is the normal way any code in QEMU might want
to create directories".

For the qtest tests, I currently ignore Coverity
reports in our test code unless it seems particularly
worthwhile to fix them. This is especially true for
complaints about unchecked return values and the like.

Even in a test case it is still not great to call
g_assert(), because this makes the test binary crash,
rather than reporting an error. The surrounding TAP
protocol parsing code then doesn't report the test
failure the way you might like.

thanks
-- PMM

Reply via email to