* [PATCH] tools/start-vmm: Add additional warning options
@ 2025-09-18 19:06 Demi Marie Obenour
2025-09-19 11:55 ` Alyssa Ross
2025-09-23 18:54 ` Alyssa Ross
0 siblings, 2 replies; 10+ messages in thread
From: Demi Marie Obenour @ 2025-09-18 19:06 UTC (permalink / raw)
To: Spectrum OS Development; +Cc: Alyssa Ross, Demi Marie Obenour
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);
---
base-commit: edd53bb5dd9b682fec744d6df7921b64e0f56565
change-id: 20250918-more-c-warnings-1-239a405997c0
--
Sincerely,
Demi Marie Obenour (she/her/hers)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-18 19:06 [PATCH] tools/start-vmm: Add additional warning options Demi Marie Obenour
@ 2025-09-19 11:55 ` Alyssa Ross
2025-09-19 19:38 ` Demi Marie Obenour
2025-09-23 18:54 ` Alyssa Ross
1 sibling, 1 reply; 10+ messages in thread
From: Alyssa Ross @ 2025-09-19 11:55 UTC (permalink / raw)
To: Demi Marie Obenour; +Cc: Spectrum OS Development
[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-19 11:55 ` Alyssa Ross
@ 2025-09-19 19:38 ` Demi Marie Obenour
2025-09-21 8:55 ` Alyssa Ross
0 siblings, 1 reply; 10+ messages in thread
From: Demi Marie Obenour @ 2025-09-19 19:38 UTC (permalink / raw)
To: Alyssa Ross; +Cc: Spectrum OS Development
[-- Attachment #1.1.1: Type: text/plain, Size: 1551 bytes --]
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.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-19 19:38 ` Demi Marie Obenour
@ 2025-09-21 8:55 ` Alyssa Ross
2025-09-21 15:57 ` Demi Marie Obenour
0 siblings, 1 reply; 10+ messages in thread
From: Alyssa Ross @ 2025-09-21 8:55 UTC (permalink / raw)
To: Demi Marie Obenour; +Cc: Spectrum OS Development
[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]
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?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-21 8:55 ` Alyssa Ross
@ 2025-09-21 15:57 ` Demi Marie Obenour
2025-09-21 16:13 ` Alyssa Ross
0 siblings, 1 reply; 10+ messages in thread
From: Demi Marie Obenour @ 2025-09-21 15:57 UTC (permalink / raw)
To: Alyssa Ross; +Cc: Spectrum OS Development
[-- Attachment #1.1.1: Type: text/plain, Size: 1933 bytes --]
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.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-21 15:57 ` Demi Marie Obenour
@ 2025-09-21 16:13 ` Alyssa Ross
2025-09-21 16:14 ` Demi Marie Obenour
0 siblings, 1 reply; 10+ messages in thread
From: Alyssa Ross @ 2025-09-21 16:13 UTC (permalink / raw)
To: Demi Marie Obenour; +Cc: Spectrum OS Development
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-21 16:13 ` Alyssa Ross
@ 2025-09-21 16:14 ` Demi Marie Obenour
2025-09-21 16:17 ` Alyssa Ross
0 siblings, 1 reply; 10+ messages in thread
From: Demi Marie Obenour @ 2025-09-21 16:14 UTC (permalink / raw)
To: Alyssa Ross; +Cc: Spectrum OS Development
[-- Attachment #1.1.1: Type: text/plain, Size: 2833 bytes --]
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.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-21 16:14 ` Demi Marie Obenour
@ 2025-09-21 16:17 ` Alyssa Ross
2025-09-21 16:18 ` Demi Marie Obenour
0 siblings, 1 reply; 10+ messages in thread
From: Alyssa Ross @ 2025-09-21 16:17 UTC (permalink / raw)
To: Demi Marie Obenour; +Cc: Spectrum OS Development
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-21 16:17 ` Alyssa Ross
@ 2025-09-21 16:18 ` Demi Marie Obenour
0 siblings, 0 replies; 10+ messages in thread
From: Demi Marie Obenour @ 2025-09-21 16:18 UTC (permalink / raw)
To: Alyssa Ross; +Cc: Spectrum OS Development
[-- Attachment #1.1.1: Type: text/plain, Size: 3309 bytes --]
On 9/21/25 12:17, Alyssa Ross wrote:
> 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?
Sure!
--
Sincerely,
Demi Marie Obenour (she/her/hers)
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tools/start-vmm: Add additional warning options
2025-09-18 19:06 [PATCH] tools/start-vmm: Add additional warning options Demi Marie Obenour
2025-09-19 11:55 ` Alyssa Ross
@ 2025-09-23 18:54 ` Alyssa Ross
1 sibling, 0 replies; 10+ messages in thread
From: Alyssa Ross @ 2025-09-23 18:54 UTC (permalink / raw)
To: Demi Marie Obenour, Spectrum OS Development
Cc: Alyssa Ross, Demi Marie Obenour
This patch has been committed as c451c3da2162fac4c6c2327a6e8d503de8d3eb16,
which can be viewed online at
https://spectrum-os.org/git/spectrum/commit/?id=c451c3da2162fac4c6c2327a6e8d503de8d3eb16.
This is an automated message. Send comments/questions/requests to:
Alyssa Ross <hi@alyssa.is>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-09-23 18:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18 19:06 [PATCH] tools/start-vmm: Add additional warning options Demi Marie Obenour
2025-09-19 11:55 ` Alyssa Ross
2025-09-19 19:38 ` Demi Marie Obenour
2025-09-21 8:55 ` Alyssa Ross
2025-09-21 15:57 ` Demi Marie Obenour
2025-09-21 16:13 ` Alyssa Ross
2025-09-21 16:14 ` Demi Marie Obenour
2025-09-21 16:17 ` Alyssa Ross
2025-09-21 16:18 ` Demi Marie Obenour
2025-09-23 18:54 ` Alyssa Ross
Code repositories for project(s) associated with this public inbox
https://spectrum-os.org/git/crosvm
https://spectrum-os.org/git/doc
https://spectrum-os.org/git/mktuntap
https://spectrum-os.org/git/nixpkgs
https://spectrum-os.org/git/spectrum
https://spectrum-os.org/git/ucspi-vsock
https://spectrum-os.org/git/www
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).