[PATCH] gendwarfksyms: make -T symtypes output be in name order

[PATCH] gendwarfksyms: make -T symtypes output be in name order

[Giuliano Procida]

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@google.com>
---
 scripts/gendwarfksyms/types.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

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 *));
+	hash_for_each_safe(type_map, e, tmp, hash)
+		es[i++] = e;
+
+	qsort(es, count, sizeof(struct type_expansion *), cmp_expansion_name);
+
+	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.rc2.761.g2dc52ea45b-goog

Re: [PATCH] gendwarfksyms: make -T symtypes output be in name order

[Greg Kroah-Hartman]

On Mon, Jun 23, 2025 at 10:21:47AM +0100, Giuliano Procida 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.

Ah, very nice, thanks!

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH] gendwarfksyms: make -T symtypes output be in name order

[Sami Tolvanen]

Hi Giuliano,

On Mon, Jun 23, 2025 at 2:26 AM Giuliano Procida <gprocida@google.com> 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@google.com>
> ---
>  scripts/gendwarfksyms/types.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> 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 *));
> +       hash_for_each_safe(type_map, e, tmp, hash)
> +               es[i++] = e;
> +
> +       qsort(es, count, sizeof(struct type_expansion *), cmp_expansion_name);
> +
> +       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);

Looks like a nice improvement. Thanks for the patch!

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami

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

[Giuliano Procida]

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@google.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
---
 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 *));
+	hash_for_each_safe(type_map, e, tmp, hash)
+		es[i++] = e;
+
+	qsort(es, count, sizeof(struct type_expansion *), cmp_expansion_name);
+
+	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

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

[Masahiro Yamada]

On Wed, Jun 25, 2025 at 6:52 PM Giuliano Procida <gprocida@google.com> 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@google.com>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> ---
>  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);



> +
> +       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

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

[Giuliano Procida]

Hi.

On Sun, 29 Jun 2025 at 18:51, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Jun 25, 2025 at 6:52 PM Giuliano Procida <gprocida@google.com> 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@google.com>
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> > ---
> >  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

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

[Masahiro Yamada]

On Mon, Jun 30, 2025 at 7:05 PM Giuliano Procida <gprocida@google.com> wrote:
>
> Hi.
>
> On Sun, 29 Jun 2025 at 18:51, Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Wed, Jun 25, 2025 at 6:52 PM Giuliano Procida <gprocida@google.com> 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@google.com>
> > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> > > ---
> > >  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

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

[Giuliano Procida]

On Mon, 30 Jun 2025 at 14:24, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Mon, Jun 30, 2025 at 7:05 PM Giuliano Procida <gprocida@google.com> wrote:
> >
> > Hi.
> >
> > On Sun, 29 Jun 2025 at 18:51, Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Wed, Jun 25, 2025 at 6:52 PM Giuliano Procida <gprocida@google.com> 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@google.com>
> > > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> > > > ---
> > > >  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
>
>

I can follow up with a change that adjusts all occurrences. That
shouldn't take long at all.

>
> --
> Best Regards
> Masahiro Yamada

[PATCH] gendwarfksyms: use preferred form of sizeof for allocation

[Giuliano Procida]

The preferred form is to supply the variable being allocated to rather
than an explicit type name which might become stale.

Also do this for memset and qsort arguments.

Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 scripts/gendwarfksyms/cache.c   | 2 +-
 scripts/gendwarfksyms/die.c     | 4 ++--
 scripts/gendwarfksyms/dwarf.c   | 2 +-
 scripts/gendwarfksyms/kabi.c    | 2 +-
 scripts/gendwarfksyms/symbols.c | 2 +-
 scripts/gendwarfksyms/types.c   | 8 ++++----
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/gendwarfksyms/cache.c b/scripts/gendwarfksyms/cache.c
index c9c19b86a686..1c640db93db3 100644
--- a/scripts/gendwarfksyms/cache.c
+++ b/scripts/gendwarfksyms/cache.c
@@ -15,7 +15,7 @@ void cache_set(struct cache *cache, unsigned long key, int value)
 {
 	struct cache_item *ci;
 
-	ci = xmalloc(sizeof(struct cache_item));
+	ci = xmalloc(sizeof(*ci));
 	ci->key = key;
 	ci->value = value;
 	hash_add(cache->cache, &ci->hash, hash_32(key));
diff --git a/scripts/gendwarfksyms/die.c b/scripts/gendwarfksyms/die.c
index 6183bbbe7b54..052f7a3f975a 100644
--- a/scripts/gendwarfksyms/die.c
+++ b/scripts/gendwarfksyms/die.c
@@ -33,7 +33,7 @@ static struct die *create_die(Dwarf_Die *die, enum die_state state)
 {
 	struct die *cd;
 
-	cd = xmalloc(sizeof(struct die));
+	cd = xmalloc(sizeof(*cd));
 	init_die(cd);
 	cd->addr = (uintptr_t)die->addr;
 
@@ -123,7 +123,7 @@ static struct die_fragment *append_item(struct die *cd)
 {
 	struct die_fragment *df;
 
-	df = xmalloc(sizeof(struct die_fragment));
+	df = xmalloc(sizeof(*df));
 	df->type = FRAGMENT_EMPTY;
 	list_add_tail(&df->list, &cd->fragments);
 	return df;
diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
index 13ea7bf1ae7d..3538a7d9cb07 100644
--- a/scripts/gendwarfksyms/dwarf.c
+++ b/scripts/gendwarfksyms/dwarf.c
@@ -634,7 +634,7 @@ static int get_union_kabi_status(Dwarf_Die *die, Dwarf_Die *placeholder,
 	 * Note that the user of this feature is responsible for ensuring
 	 * that the structure actually remains ABI compatible.
 	 */
-	memset(&state.kabi, 0, sizeof(struct kabi_state));
+	memset(&state.kabi, 0, sizeof(state.kabi));
 
 	res = checkp(process_die_container(&state, NULL, die,
 					   check_union_member_kabi_status,
diff --git a/scripts/gendwarfksyms/kabi.c b/scripts/gendwarfksyms/kabi.c
index b3ade713778f..e3c2a3ccf51a 100644
--- a/scripts/gendwarfksyms/kabi.c
+++ b/scripts/gendwarfksyms/kabi.c
@@ -228,7 +228,7 @@ void kabi_read_rules(int fd)
 		if (type == KABI_RULE_TYPE_UNKNOWN)
 			error("unsupported kABI rule type: '%s'", field);
 
-		rule = xmalloc(sizeof(struct rule));
+		rule = xmalloc(sizeof(*rule));
 
 		rule->type = type;
 		rule->target = xstrdup(get_rule_field(&rule_str, &left));
diff --git a/scripts/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
index 327f87389c34..35ed594f0749 100644
--- a/scripts/gendwarfksyms/symbols.c
+++ b/scripts/gendwarfksyms/symbols.c
@@ -146,7 +146,7 @@ void symbol_read_exports(FILE *file)
 			continue;
 		}
 
-		sym = xcalloc(1, sizeof(struct symbol));
+		sym = xcalloc(1, sizeof(*sym));
 		sym->name = name;
 		sym->addr.section = SHN_UNDEF;
 		sym->state = SYMBOL_UNPROCESSED;
diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
index 51c1471e8684..9c3b053bf061 100644
--- a/scripts/gendwarfksyms/types.c
+++ b/scripts/gendwarfksyms/types.c
@@ -45,7 +45,7 @@ static int type_list_append(struct list_head *list, const char *s, void *owned)
 	if (!s)
 		return 0;
 
-	entry = xmalloc(sizeof(struct type_list_entry));
+	entry = xmalloc(sizeof(*entry));
 	entry->str = s;
 	entry->owned = owned;
 	list_add_tail(&entry->list, list);
@@ -122,7 +122,7 @@ static struct type_expansion *type_map_add(const char *name,
 	struct type_expansion *e;
 
 	if (__type_map_get(name, &e)) {
-		e = xmalloc(sizeof(struct type_expansion));
+		e = xmalloc(sizeof(*e));
 		type_expansion_init(e);
 		e->name = xstrdup(name);
 
@@ -202,11 +202,11 @@ static void type_map_write(FILE *file)
 
 	hash_for_each_safe(type_map, e, tmp, hash)
 		++count;
-	es = xmalloc(count * sizeof(struct type_expansion *));
+	es = xmalloc(count * sizeof(*es));
 	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);
 
 	for (i = 0; i < count; ++i) {
 		checkp(fputs(es[i]->name, file));
-- 
2.50.0.727.gbf7dc18ff4-goog

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

[Masahiro Yamada]

On Mon, Jun 30, 2025 at 10:46 PM Giuliano Procida <gprocida@google.com> wrote:
>
> On Mon, 30 Jun 2025 at 14:24, Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Mon, Jun 30, 2025 at 7:05 PM Giuliano Procida <gprocida@google.com> wrote:
> > >
> > > Hi.
> > >
> > > On Sun, 29 Jun 2025 at 18:51, Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > On Wed, Jun 25, 2025 at 6:52 PM Giuliano Procida <gprocida@google.com> 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@google.com>
> > > > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> > > > > ---
> > > > >  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
> >
> >
>
> I can follow up with a change that adjusts all occurrences. That
> shouldn't take long at all.

I expected a new patch version (I do not know whether it is v2 or v3 since
you do not add such a prefix),
instead of breaking the style, and fixing it in a follow-up patch.







--
Best Regards
Masahiro Yamada