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

[Alyssa Ross <hi@alyssa.is>]

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

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

> On 9/21/25 12:13, Alyssa Ross wrote:
>> 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.
>
> It also catches cases where a function or variable should have been marked
> static.

Okay, I /am/ bad at remembering to mark functions as static, and I guess
there's not really a better way to avoid that, so happy to take it with
that rationale.  Happy for me to just adjust the commit message to
mention that?

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