Re: [PATCH v2] pahole: fix discarded-qualifiers for strchr/strstr.
Alan Maguire <[email protected]>
| Newsgroups | org.kernel.vger.dwarves |
|---|---|
| Message-ID | <[email protected]> |
On 09/03/2026 16:52, d.hourtoulle wrote: > From: Damien Hourtoulle <[email protected]> > > Glibc 2.43 added C23 const-preserving overloads : > https://sourceware.org/glibc/wiki/Release/2.43. > > For the function prototype__new, fix local variable declaration to use > the correct const char* type instead of char*, removing the need to > discard the const qualifier. > > Signed-off-by: Damien Hourtoulle <[email protected]> the changes look good, but there is an existing issue in the code that would be good to solve while you're in the neighbourhood I think. more below.. > --- > Changes in v2: > - In type_instance__int_value loop: update sep_mutable instead of sep > - In type__find_type_enum loop: introduce char *cur to iterate through > the mutable copy, so sep_mutable stays const-correct and is updated > correctly on each iteration > > btf_encoder.c | 2 +- > pahole.c | 33 ++++++++++++++++----------------- > 2 files changed, 17 insertions(+), 18 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index aa7cd1c..d36984a 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -1218,7 +1218,7 @@ static bool str_contains_non_fn_suffix(const char *str) { > ".cold", > ".part" > }; > - char *suffix = strchr(str, '.'); > + const char *suffix = strchr(str, '.'); > int i; > > if (!suffix) > diff --git a/pahole.c b/pahole.c > index 02a0d19..9dfeeb2 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -2483,7 +2483,7 @@ static int64_t type_instance__int_value(struct type_instance *instance, const ch > int byte_offset = 0; > > if (!member) { > - char *sep = strchr(member_name_orig, '.'); > + const char *sep = strchr(member_name_orig, '.'); > > if (!sep) > return -1; > @@ -2496,8 +2496,8 @@ static int64_t type_instance__int_value(struct type_instance *instance, const ch > char *member_name = member_name_alloc; > struct type *type = instance->type; > > - sep = member_name_alloc + (sep - member_name_orig); > - *sep = 0; > + char *sep_mutable = member_name_alloc + (sep - member_name_orig); // sep mutable for the copy > + *sep_mutable = 0; > > while (1) { > member = type__find_member_by_name(type, member_name); > @@ -2510,9 +2510,9 @@ out_free_member_name: > type = tag__type(cu__type(cu, member->tag.type)); > if (type == NULL) > goto out_free_member_name; > - member_name = sep + 1; > - sep = strchr(member_name, '.'); > - if (!sep) > + member_name = sep_mutable + 1; > + sep_mutable = strchr(member_name, '.'); > + if (!sep_mutable) > break; > > } > @@ -2950,7 +2950,7 @@ static struct prototype *prototype__new(const char *expression) > > strcpy(prototype->name, expression); > > - const char *name = prototype->name; > + char *name = prototype->name; > > prototype->nr_args = 0; > > @@ -2964,10 +2964,9 @@ static struct prototype *prototype__new(const char *expression) > if (args_close == NULL) > goto out_no_closing_parens; > > + *args_open++ = *args_close = '\0'; > char *args = args_open; > > - *args++ = *args_close = '\0'; > - > while (isspace(*args)) > ++args; > > @@ -3114,7 +3113,7 @@ static int type__find_type_enum(struct type *type, struct cu *cu, const char *ty > return type__add_type_enum(type, te, cu); > > // Now look at a 'virtual enum', i.e. the concatenation of multiple enums > - char *sep = strchr(type_enum, '+'); > + const char *sep = strchr(type_enum, '+'); > > if (!sep) > return -1; > @@ -3126,13 +3125,13 @@ static int type__find_type_enum(struct type *type, struct cu *cu, const char *ty > > int ret = -1; > > - sep = type_enums + (sep - type_enum); > + char *sep_mutable = type_enums + (sep - type_enum); > + char *cur = type_enums; > > - type_enum = type_enums; > - *sep = '\0'; > + *sep_mutable = '\0'; > > while (1) { > - te = cu__find_enumeration_by_name(cu, type_enum, NULL); > + te = cu__find_enumeration_by_name(cu, cur, NULL); > > if (!te) > goto out; > @@ -3141,10 +3140,10 @@ static int type__find_type_enum(struct type *type, struct cu *cu, const char *ty > if (ret) > goto out; > > - if (sep == NULL) > + if (sep_mutable == NULL) > break; > - type_enum = sep + 1; > - sep = strchr(type_enum, '+'); > + cur = sep_mutable + 1; > + sep_mutable = strchr(cur, '+'); > } > > ret = 0; the intent of this loop is to search through a "+" concatenated list of enum names, but the problem is we never nullify sep_mutable (or sep in the old code) at the end of each loop iteration. so when we find the enumeration by name at the top of the second iteration of the while() loop were searching with "second+third" rather than "second". So long story short, we need a sep_mutable = '\0'; ...at the end of the while loop I think.