[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qtests: avoid printing comments before g_test_init()
From: |
Juan Quintela |
Subject: |
Re: [PATCH] qtests: avoid printing comments before g_test_init() |
Date: |
Fri, 24 Mar 2023 18:29:27 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Thomas Huth <thuth@redhat.com> wrote:
> On 23/03/2023 19.31, Juan Quintela wrote:
>> Daniel P. Berrangé <berrange@redhat.com> wrote:
>>> The TAP protocol version line must be the first thing printed on
>>> stdout. The migration test failed that requirement in certain
>>> scenarios:
>>>
>>> # Skipping test: Userfault not available (builtdtime)
>>> TAP version 13
>>> # random seed: R02Sc120c807f11053eb90bfea845ba1e368
>>> 1..32
>>> # Start of x86_64 tests
>>> # Start of migration tests
>>> ....
>>>
>>> The TAP version is printed by g_test_init(), so we need to make
>>> sure that any methods which print are run after that.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>
>>> - const bool has_kvm = qtest_has_accel("kvm");
>>> - const bool has_uffd = ufd_version_check();
>>> - const char *arch = qtest_get_arch();
>>> + bool has_kvm;
>>> + bool has_uffd;
>>> + const char *arch;
>> Why don't you move also the declarations of the variables?
>> I think that one of the biggest troubles of C is variables that are not
>> initialized.
>> All compilers that we support are C99 or later, so we can do that
>> (and
>> we already do in lot of places.)
>
> I think the coding style has been created before we switched to
> -std=gnu99 for compiling QEMU, so a lot of GCCs were still using C89
> by default?
Yes, that is the actitude.
I got sick when I see new code that still does:
char *foo = (char *)malloc(...);
It is is C89, it has been enough to know that it is not needed.
And yes, that particular one is not used in qemu anymore, but:
void *opaque;
....
Foo *foo = (Foo *)opaque;
Is still introduced in new code, and it is not needed since C89.
>> And yeap, I know that CodingStyle says otherwise, but I think that what
>> is wrong is CodingStyle.
>> https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg03836.html
>
> Please use proper prefixes in the subject when sending patches
> ("docs/devel:" here), otherwise your patches might not get the right
> attention (at least on my side, it was filtered away as a patch that
> was relevant to me) - and also put some recent contributors on CC:
I didn't knew the docs/devel preffix.
About the CC'd, I expected that git-publish be good enough at doing
that, but it appears not.
Anyways, thanks.
Later, Juan.