On Mon, Jun 30, 2025 at 7:05 PM Giuliano Procida <gprocida@xxxxxxxxxx> wrote:
>
> Hi.
>
> On Sun, 29 Jun 2025 at 18:51, Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> >
> > On Wed, Jun 25, 2025 at 6:52 PM Giuliano Procida <gprocida@xxxxxxxxxx> wrote:
> > >
> > > When writing symtypes information, we iterate through the entire hash
> > > table containing type expansions. The key order varies unpredictably
> > > as new entries are added, making it harder to compare symtypes between
> > > builds.
> > >
> > > Resolve this by sorting the type expansions by name before output.
> > >
> > > Signed-off-by: Giuliano Procida <gprocida@xxxxxxxxxx>
> > > Acked-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Reviewed-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> > > ---
> > > scripts/gendwarfksyms/types.c | 29 ++++++++++++++++++++++++++---
> > > 1 file changed, 26 insertions(+), 3 deletions(-)
> > >
> > > [Adjusted the first line of the description. Added reviewer tags.
> > > Added missing CC to linux-modules.]
> > >
> > > diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
> > > index 7bd459ea6c59..51c1471e8684 100644
> > > --- a/scripts/gendwarfksyms/types.c
> > > +++ b/scripts/gendwarfksyms/types.c
> > > @@ -6,6 +6,8 @@
> > > #define _GNU_SOURCE
> > > #include <inttypes.h>
> > > #include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > #include <zlib.h>
> > >
> > > #include "gendwarfksyms.h"
> > > @@ -179,20 +181,41 @@ static int type_map_get(const char *name, struct type_expansion **res)
> > > return -1;
> > > }
> > >
> > > +static int cmp_expansion_name(const void *p1, const void *p2)
> > > +{
> > > + struct type_expansion *const *e1 = p1;
> > > + struct type_expansion *const *e2 = p2;
> > > +
> > > + return strcmp((*e1)->name, (*e2)->name);
> > > +}
> > > +
> > > static void type_map_write(FILE *file)
> > > {
> > > struct type_expansion *e;
> > > struct hlist_node *tmp;
> > > + struct type_expansion **es;
> > > + size_t count = 0;
> > > + size_t i = 0;
> > >
> > > if (!file)
> > > return;
> > >
> > > - hash_for_each_safe(type_map, e, tmp, hash) {
> > > - checkp(fputs(e->name, file));
> > > + hash_for_each_safe(type_map, e, tmp, hash)
> > > + ++count;
> > > + es = xmalloc(count * sizeof(struct type_expansion *));
> >
> > Just a nit:
> >
> > es = xmalloc(count * sizeof(*es));
> >
> > is better?
> >
> > > + hash_for_each_safe(type_map, e, tmp, hash)
> > > + es[i++] = e;
> > > +
> > > + qsort(es, count, sizeof(struct type_expansion *), cmp_expansion_name);
> >
> > qsort(es, count, sizeof(*es), cmp_expansion_name);
> >
>
> That's a fair point.
>
> However, in the gendwarfksyms code, all but one of the sizeofs uses an
> explicit type name. The exception is sizeof(stats) where stats is an array.
>
> I'll leave Sami's code as it is.
This rule is clearly documented with rationale.
See this:
https://github.com/torvalds/linux/blob/v6.15/Documentation/process/coding-style.rst?plain=1#L941
--
Best Regards
Masahiro Yamada
