Re: [PATCH] tools/start-vmm: Add additional warning options

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 9/21/25 04:55, Alyssa Ross wrote:
>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>> 
>>> On 9/19/25 07:55, Alyssa Ross wrote:
>>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>>
>>>>> This detected a missing prototype.
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>>>> ---
>>>>>  tools/meson.build    | 2 ++
>>>>>  tools/start-vmm/ch.h | 1 +
>>>>>  2 files changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/tools/meson.build b/tools/meson.build
>>>>> index 9cebd03e323531fca7600cacf120161a98de16c5..8262f3e01d7bd56561306d7dd4650a22ca40ebe7 100644
>>>>> --- a/tools/meson.build
>>>>> +++ b/tools/meson.build
>>>>> @@ -9,6 +9,8 @@ project('spectrum-tools', 'c',
>>>>>    })
>>>>>  
>>>>>  add_project_arguments('-Wno-error=attributes', language : 'c')
>>>>> +add_project_arguments('-Werror=missing-prototypes', language : 'c')
>>>>> +add_project_arguments('-Werror=missing-declarations', language : 'c')
>>>>>  
>>>>>  if get_option('host')
>>>>>    add_languages('rust')
>>>>> diff --git a/tools/start-vmm/ch.h b/tools/start-vmm/ch.h
>>>>> index 7230913ef0abf41a4f712ac4a543c7f7fdecec0f..5431365e6e2894cdebae22a9a44e2ccf1222e0d2 100644
>>>>> --- a/tools/start-vmm/ch.h
>>>>> +++ b/tools/start-vmm/ch.h
>>>>> @@ -8,3 +8,4 @@ struct net_config {
>>>>>  	char id[18];
>>>>>  	uint8_t mac[6];
>>>>>  };
>>>>> +struct net_config net_setup(const char name[static 1], int name_len);
>>>>
>>>> Why do we need to declare this in a C header?  It's only used from Rust.
>>>
>>> Ideally the Rust declarations would be generated from the C ones
>>> using bindgen.  Also, this catches genuine bugs on the C side.
>> 
>> What bugs?  Implicit definitions are already disallowed, aren't they?
>
> Function declared in one file, defined in another file with different
> prototype.  This makes it undefined behavior to call.

I don't see how this really enforces that?  Fundamentally the problem
there is conflicting prototypes, not missing ones.  If you had another
compilation unit that didn't include the header with the prototype, and
instead make its own declaration, you'd still have the same problem.  If
we have a norm of not declaring functions outside of the header file
that corresponds to and is included by the implementation file, that
prevents that from happening regardless of whether we have these errors,
assuming that the compiler would still catch the prototype not matching
the implementation.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]