Re: [PATCH] gendwarfksyms: order -T symtypes output by name

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Giuliano.

>
> > +
> > +       for (i = 0; i < count; ++i) {
> > +               checkp(fputs(es[i]->name, file));
> >                 checkp(fputs(" ", file));
> > -               type_list_write(&e->expanded, file);
> > +               type_list_write(&es[i]->expanded, file);
> >                 checkp(fputs("\n", file));
> >         }
> > +
> > +       free(es);
> >  }
> >
> >  static void type_map_free(void)
> > --
> > 2.50.0.714.g196bf9f422-goog
> >
> >
>
>
> --
> Best Regards
> Masahiro Yamada





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux