[PATCH 7/8] symbols: drop symbols-dummy.c

Jan Beulich posted 8 patches 2 weeks, 3 days ago
[PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Jan Beulich 2 weeks, 3 days ago
No architecture using it anymore, we can as well get rid of it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Should we also drop common/symbols.h again then, by moving its contents
back into common/symbols.c?

--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -74,8 +74,6 @@ ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
 obj-y += domctl.o
 endif
 
-extra-y := symbols-dummy.o
-
 obj-$(CONFIG_COVERAGE) += coverage/
 obj-y += sched/
 obj-$(CONFIG_UBSAN) += ubsan/
--- a/xen/common/symbols-dummy.c
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * symbols-dummy.c: dummy symbol-table definitions for the inital partial
- *                  link of the hypervisor image.
- */
-
-#include "symbols.h"
-
-#ifdef SYMBOLS_ORIGIN
-const unsigned int symbols_offsets[1];
-#else
-const unsigned long symbols_addresses[1];
-#endif
-const unsigned int symbols_num_addrs;
-const unsigned char symbols_names[1];
-
-#ifdef CONFIG_FAST_SYMBOL_LOOKUP
-const unsigned int symbols_num_names;
-const struct symbol_offset symbols_sorted_offsets[1];
-#endif
-
-const uint8_t symbols_token_table[1];
-const uint16_t symbols_token_index[1];
-
-const unsigned int symbols_markers[1];
Re: [PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Jan Beulich 5 days, 17 hours ago
On 26.11.2025 14:47, Jan Beulich wrote:
> No architecture using it anymore, we can as well get rid of it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Should we also drop common/symbols.h again then, by moving its contents
> back into common/symbols.c?
> 
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -74,8 +74,6 @@ ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
>  obj-y += domctl.o
>  endif
>  
> -extra-y := symbols-dummy.o
> -
>  obj-$(CONFIG_COVERAGE) += coverage/
>  obj-y += sched/
>  obj-$(CONFIG_UBSAN) += ubsan/
> --- a/xen/common/symbols-dummy.c
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/*
> - * symbols-dummy.c: dummy symbol-table definitions for the inital partial
> - *                  link of the hypervisor image.
> - */
> -
> -#include "symbols.h"
> -
> -#ifdef SYMBOLS_ORIGIN
> -const unsigned int symbols_offsets[1];
> -#else
> -const unsigned long symbols_addresses[1];
> -#endif
> -const unsigned int symbols_num_addrs;
> -const unsigned char symbols_names[1];
> -
> -#ifdef CONFIG_FAST_SYMBOL_LOOKUP
> -const unsigned int symbols_num_names;
> -const struct symbol_offset symbols_sorted_offsets[1];
> -#endif
> -
> -const uint8_t symbols_token_table[1];
> -const uint16_t symbols_token_index[1];
> -
> -const unsigned int symbols_markers[1];
> 

Now this is (to me at least) absurd: I'm removing a file, just to find the pipeline
fails because cppcheck doesn't like docs/misra/exclude-list.json containing a
reference to a non-existing file.

I'll amend the commit with

--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -170,10 +170,6 @@
             "comment": "Imported from Linux, ignore for now"
         },
         {
-            "rel_path": "common/symbols-dummy.c",
-            "comment": "The resulting code is not included in the final Xen binary, ignore for now"
-        },
-        {
             "rel_path": "crypto/*",
             "comment": "Origin is external and documented in crypto/README.source"
         },

but I think such tidying should be optional.

Jan
Re: [PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Luca Fancellu 5 days, 17 hours ago
Hi Jan,

> On 8 Dec 2025, at 14:48, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 26.11.2025 14:47, Jan Beulich wrote:
>> No architecture using it anymore, we can as well get rid of it.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Should we also drop common/symbols.h again then, by moving its contents
>> back into common/symbols.c?
>> 
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -74,8 +74,6 @@ ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
>> obj-y += domctl.o
>> endif
>> 
>> -extra-y := symbols-dummy.o
>> -
>> obj-$(CONFIG_COVERAGE) += coverage/
>> obj-y += sched/
>> obj-$(CONFIG_UBSAN) += ubsan/
>> --- a/xen/common/symbols-dummy.c
>> +++ /dev/null
>> @@ -1,24 +0,0 @@
>> -/*
>> - * symbols-dummy.c: dummy symbol-table definitions for the inital partial
>> - *                  link of the hypervisor image.
>> - */
>> -
>> -#include "symbols.h"
>> -
>> -#ifdef SYMBOLS_ORIGIN
>> -const unsigned int symbols_offsets[1];
>> -#else
>> -const unsigned long symbols_addresses[1];
>> -#endif
>> -const unsigned int symbols_num_addrs;
>> -const unsigned char symbols_names[1];
>> -
>> -#ifdef CONFIG_FAST_SYMBOL_LOOKUP
>> -const unsigned int symbols_num_names;
>> -const struct symbol_offset symbols_sorted_offsets[1];
>> -#endif
>> -
>> -const uint8_t symbols_token_table[1];
>> -const uint16_t symbols_token_index[1];
>> -
>> -const unsigned int symbols_markers[1];
>> 
> 
> Now this is (to me at least) absurd: I'm removing a file, just to find the pipeline
> fails because cppcheck doesn't like docs/misra/exclude-list.json containing a
> reference to a non-existing file.
> 
> I'll amend the commit with
> 
> --- a/docs/misra/exclude-list.json
> +++ b/docs/misra/exclude-list.json
> @@ -170,10 +170,6 @@
>             "comment": "Imported from Linux, ignore for now"
>         },
>         {
> -            "rel_path": "common/symbols-dummy.c",
> -            "comment": "The resulting code is not included in the final Xen binary, ignore for now"
> -        },
> -        {
>             "rel_path": "crypto/*",
>             "comment": "Origin is external and documented in crypto/README.source"
>         },
> 
> but I think such tidying should be optional.
> 
> Jan

Can you share the error? 

Cheers,
Luca
Re: [PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Jan Beulich 5 days, 17 hours ago
On 08.12.2025 15:51, Luca Fancellu wrote:
> Hi Jan,
> 
>> On 8 Dec 2025, at 14:48, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.11.2025 14:47, Jan Beulich wrote:
>>> No architecture using it anymore, we can as well get rid of it.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> Should we also drop common/symbols.h again then, by moving its contents
>>> back into common/symbols.c?
>>>
>>> --- a/xen/common/Makefile
>>> +++ b/xen/common/Makefile
>>> @@ -74,8 +74,6 @@ ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
>>> obj-y += domctl.o
>>> endif
>>>
>>> -extra-y := symbols-dummy.o
>>> -
>>> obj-$(CONFIG_COVERAGE) += coverage/
>>> obj-y += sched/
>>> obj-$(CONFIG_UBSAN) += ubsan/
>>> --- a/xen/common/symbols-dummy.c
>>> +++ /dev/null
>>> @@ -1,24 +0,0 @@
>>> -/*
>>> - * symbols-dummy.c: dummy symbol-table definitions for the inital partial
>>> - *                  link of the hypervisor image.
>>> - */
>>> -
>>> -#include "symbols.h"
>>> -
>>> -#ifdef SYMBOLS_ORIGIN
>>> -const unsigned int symbols_offsets[1];
>>> -#else
>>> -const unsigned long symbols_addresses[1];
>>> -#endif
>>> -const unsigned int symbols_num_addrs;
>>> -const unsigned char symbols_names[1];
>>> -
>>> -#ifdef CONFIG_FAST_SYMBOL_LOOKUP
>>> -const unsigned int symbols_num_names;
>>> -const struct symbol_offset symbols_sorted_offsets[1];
>>> -#endif
>>> -
>>> -const uint8_t symbols_token_table[1];
>>> -const uint16_t symbols_token_index[1];
>>> -
>>> -const unsigned int symbols_markers[1];
>>>
>>
>> Now this is (to me at least) absurd: I'm removing a file, just to find the pipeline
>> fails because cppcheck doesn't like docs/misra/exclude-list.json containing a
>> reference to a non-existing file.
>>
>> I'll amend the commit with
>>
>> --- a/docs/misra/exclude-list.json
>> +++ b/docs/misra/exclude-list.json
>> @@ -170,10 +170,6 @@
>>             "comment": "Imported from Linux, ignore for now"
>>         },
>>         {
>> -            "rel_path": "common/symbols-dummy.c",
>> -            "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>> -        },
>> -        {
>>             "rel_path": "crypto/*",
>>             "comment": "Origin is external and documented in crypto/README.source"
>>         },
>>
>> but I think such tidying should be optional.
> 
> Can you share the error? 

+ xen/scripts/xen-analysis.py --run-cppcheck --cppcheck-misra -- -j16
ERROR: Issue with reading file /builds/xen-project/hardware/xen-staging/docs/misra/exclude-list.json: Malformed path: common/symbols-dummy.c refers to /builds/xen-project/hardware/xen-staging/xen/common/symbols-dummy.c that does not exists

Jan
Re: [PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Luca Fancellu 5 days, 16 hours ago
Hi Jan,

>>>> 
>>> 
>>> Now this is (to me at least) absurd: I'm removing a file, just to find the pipeline
>>> fails because cppcheck doesn't like docs/misra/exclude-list.json containing a
>>> reference to a non-existing file.
>>> 
>>> I'll amend the commit with
>>> 
>>> --- a/docs/misra/exclude-list.json
>>> +++ b/docs/misra/exclude-list.json
>>> @@ -170,10 +170,6 @@
>>>            "comment": "Imported from Linux, ignore for now"
>>>        },
>>>        {
>>> -            "rel_path": "common/symbols-dummy.c",
>>> -            "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>> -        },
>>> -        {
>>>            "rel_path": "crypto/*",
>>>            "comment": "Origin is external and documented in crypto/README.source"
>>>        },
>>> 
>>> but I think such tidying should be optional.
>> 
>> Can you share the error?
> 
> + xen/scripts/xen-analysis.py --run-cppcheck --cppcheck-misra -- -j16
> ERROR: Issue with reading file /builds/xen-project/hardware/xen-staging/docs/misra/exclude-list.json: Malformed path: common/symbols-dummy.c refers to /builds/xen-project/hardware/xen-staging/xen/common/symbols-dummy.c that does not exists
> 

Oh ok I see, seems to me a good feedback from the tool to keep everything consistent.

Cheers,
Luca
Re: [PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Jan Beulich 5 days, 16 hours ago
On 08.12.2025 16:19, Luca Fancellu wrote:
>>>> Now this is (to me at least) absurd: I'm removing a file, just to find the pipeline
>>>> fails because cppcheck doesn't like docs/misra/exclude-list.json containing a
>>>> reference to a non-existing file.
>>>>
>>>> I'll amend the commit with
>>>>
>>>> --- a/docs/misra/exclude-list.json
>>>> +++ b/docs/misra/exclude-list.json
>>>> @@ -170,10 +170,6 @@
>>>>            "comment": "Imported from Linux, ignore for now"
>>>>        },
>>>>        {
>>>> -            "rel_path": "common/symbols-dummy.c",
>>>> -            "comment": "The resulting code is not included in the final Xen binary, ignore for now"
>>>> -        },
>>>> -        {
>>>>            "rel_path": "crypto/*",
>>>>            "comment": "Origin is external and documented in crypto/README.source"
>>>>        },
>>>>
>>>> but I think such tidying should be optional.
>>>
>>> Can you share the error?
>>
>> + xen/scripts/xen-analysis.py --run-cppcheck --cppcheck-misra -- -j16
>> ERROR: Issue with reading file /builds/xen-project/hardware/xen-staging/docs/misra/exclude-list.json: Malformed path: common/symbols-dummy.c refers to /builds/xen-project/hardware/xen-staging/xen/common/symbols-dummy.c that does not exists
> 
> Oh ok I see, seems to me a good feedback from the tool to keep everything consistent.

Well. If I remove a file from the xen/ subtree, then seeing empty grep output on that
subtree should allow me to be sufficiently sure that no problematic (as in: breaking
the build) references have survived.

Plus while I agree that consistency is desirable to have here, is a stale file ref
really a reason to cause a build to fail? Wouldn't a warning suffice?

How many tools' behaviors can we (as developers, reviewers, and committers) keep in
mind when putting together otherwise simple changes?

Jan
Re: [PATCH 7/8] symbols: drop symbols-dummy.c
Posted by Andrew Cooper 2 weeks, 2 days ago
On 26/11/2025 1:47 pm, Jan Beulich wrote:
> No architecture using it anymore, we can as well get rid of it.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
> Should we also drop common/symbols.h again then, by moving its contents
> back into common/symbols.c?

Probably.

~Andrew