[2/5] sim/ppc: fix warnings related to printf format strings
Commit Message
This commit is a follow on to:
commit 182421c9d2eea8c4877d983a2124e591f0aca710
Date: Tue Oct 11 15:02:08 2022 +0100
sim/ppc: fixes for arguments to printf style functions
where commit 182421c9d2ee addressed issues with printf format
arguments that were causing the compiler to give an error, this commit
addresses issues that caused the compiler to emit a warning.
This commit is mostly either changing the format string to match the
argument, or in some cases, excess, unused arguments are removed.
---
sim/ppc/igen.c | 2 +-
sim/ppc/ld-cache.c | 4 ++--
sim/ppc/ld-decode.c | 2 +-
sim/ppc/ld-insn.c | 41 +++++++++++++++--------------------------
4 files changed, 19 insertions(+), 30 deletions(-)
Comments
On 2022/10/12 21:38, Andrew Burgess via Gdb-patches wrote:
> This commit is a follow on to:
>
> commit 182421c9d2eea8c4877d983a2124e591f0aca710
> Date: Tue Oct 11 15:02:08 2022 +0100
>
> sim/ppc: fixes for arguments to printf style functions
>
> where commit 182421c9d2ee addressed issues with printf format
> arguments that were causing the compiler to give an error, this commit
> addresses issues that caused the compiler to emit a warning.
>
> This commit is mostly either changing the format string to match the
> argument, or in some cases, excess, unused arguments are removed.
> ---
> sim/ppc/igen.c | 2 +-
> sim/ppc/ld-cache.c | 4 ++--
> sim/ppc/ld-decode.c | 2 +-
> sim/ppc/ld-insn.c | 41 +++++++++++++++--------------------------
> 4 files changed, 19 insertions(+), 30 deletions(-)
>
> diff --git a/sim/ppc/igen.c b/sim/ppc/igen.c
> index 6a6dbc30f31..27b48638276 100644
> --- a/sim/ppc/igen.c
> +++ b/sim/ppc/igen.c
> @@ -442,7 +442,7 @@ main(int argc,
> code |= generate_with_icache;
> break;
> default:
> - error (NULL, "Expecting -Ggen-icache or -Ggen-icache=<N>\n");
> + error ("Expecting -Ggen-icache or -Ggen-icache=<N>\n");
> }
> }
> }
> diff --git a/sim/ppc/ld-cache.c b/sim/ppc/ld-cache.c
> index f57f7db650a..b30b1f4d898 100644
> --- a/sim/ppc/ld-cache.c
> +++ b/sim/ppc/ld-cache.c
> @@ -88,13 +88,13 @@ static void
> dump_cache_rule(cache_table* rule,
> int indent)
> {
> - dumpf(indent, "((cache_table*)0x%x\n", rule);
> + dumpf(indent, "((cache_table*)0x%p\n", rule);
Replacing "0x%p" with "%p" is recommended (as I submitted earlier).
Thanks,
Tsukasa
> dumpf(indent, " (type %s)\n", i2name(rule->type, cache_type_map));
> dumpf(indent, " (field_name \"%s\")\n", rule->field_name);
> dumpf(indent, " (derived_name \"%s\")\n", rule->derived_name);
> dumpf(indent, " (type-def \"%s\")\n", rule->type_def);
> dumpf(indent, " (expression \"%s\")\n", rule->expression);
> - dumpf(indent, " (next 0x%x)\n", rule->next);
> + dumpf(indent, " (next 0x%p)\n", rule->next);
> dumpf(indent, " )\n");
> }
>
> diff --git a/sim/ppc/ld-decode.c b/sim/ppc/ld-decode.c
> index 68d9f5f4f52..83ff1216b30 100644
> --- a/sim/ppc/ld-decode.c
> +++ b/sim/ppc/ld-decode.c
> @@ -120,7 +120,7 @@ dump_decode_rule(decode_table *rule,
> dumpf(indent, " (special_mask 0x%x)\n", rule->special_mask);
> dumpf(indent, " (special_value 0x%x)\n", rule->special_value);
> dumpf(indent, " (special_constant 0x%x)\n", rule->special_constant);
> - dumpf(indent, " (next 0x%x)\n", rule->next);
> + dumpf(indent, " (next 0x%p)\n", rule->next);
> }
> dumpf(indent, " )\n");
> }
> diff --git a/sim/ppc/ld-insn.c b/sim/ppc/ld-insn.c
> index 3910af3fdf6..c89c81c1073 100644
> --- a/sim/ppc/ld-insn.c
> +++ b/sim/ppc/ld-insn.c
> @@ -827,29 +827,18 @@ static void
> dump_insn_field(insn_field *field,
> int indent)
> {
> -
> - printf("(insn_field*)0x%lx\n", (unsigned long)field);
> -
> - dumpf(indent, "(first %d)\n", field->first);
> -
> - dumpf(indent, "(last %d)\n", field->last);
> -
> - dumpf(indent, "(width %d)\n", field->width);
> -
> + printf ("(insn_field*)0x%p\n", field);
> + dumpf (indent, "(first %d)\n", field->first);
> + dumpf (indent, "(last %d)\n", field->last);
> + dumpf (indent, "(width %d)\n", field->width);
> if (field->is_int)
> - dumpf(indent, "(is_int %d)\n", field->val_int);
> -
> + dumpf (indent, "(is_int %d)\n", field->val_int);
> if (field->is_slash)
> - dumpf(indent, "(is_slash)\n");
> -
> + dumpf (indent, "(is_slash)\n");
> if (field->is_string)
> - dumpf(indent, "(is_string `%s')\n", field->val_string);
> -
> - dumpf(indent, "(next 0x%x)\n", field->next);
> -
> - dumpf(indent, "(prev 0x%x)\n", field->prev);
> -
> -
> + dumpf (indent, "(is_string `%s')\n", field->val_string);
> + dumpf (indent, "(next 0x%p)\n", field->next);
> + dumpf (indent, "(prev 0x%p)\n", field->prev);
> }
>
> static void
> @@ -860,13 +849,13 @@ dump_insn_fields(insn_fields *fields,
>
> printf("(insn_fields*)%p\n", fields);
>
> - dumpf(indent, "(first 0x%x)\n", fields->first);
> - dumpf(indent, "(last 0x%x)\n", fields->last);
> + dumpf(indent, "(first 0x%p)\n", fields->first);
> + dumpf(indent, "(last 0x%p)\n", fields->last);
>
> dumpf(indent, "(value 0x%x)\n", fields->value);
>
> for (i = 0; i < insn_bit_size; i++) {
> - dumpf(indent, "(bits[%d] ", i, fields->bits[i]);
> + dumpf(indent, "(bits[%d]", i);
> dump_insn_field(fields->bits[i], indent+1);
> dumpf(indent, " )\n");
> }
> @@ -961,16 +950,16 @@ dump_insn_table(insn_table *table,
> dump_opcode_field(table->opcode, indent+1, 1);
> dumpf(indent, " )\n");
>
> - dumpf(indent, "(nr_entries %d)\n", table->entries);
> + dumpf(indent, "(nr_entries %d)\n", table->nr_entries);
> dumpf(indent, "(entries ");
> dump_insn_table(table->entries, indent+1, table->nr_entries);
> dumpf(indent, " )\n");
>
> - dumpf(indent, "(sibling ", table->sibling);
> + dumpf(indent, "(sibling ");
> dump_insn_table(table->sibling, indent+1, levels-1);
> dumpf(indent, " )\n");
>
> - dumpf(indent, "(parent ", table->parent);
> + dumpf(indent, "(parent ");
> dump_insn_table(table->parent, indent+1, 0);
> dumpf(indent, " )\n");
>
Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> On 2022/10/12 21:38, Andrew Burgess via Gdb-patches wrote:
>> This commit is a follow on to:
>>
>> commit 182421c9d2eea8c4877d983a2124e591f0aca710
>> Date: Tue Oct 11 15:02:08 2022 +0100
>>
>> sim/ppc: fixes for arguments to printf style functions
>>
>> where commit 182421c9d2ee addressed issues with printf format
>> arguments that were causing the compiler to give an error, this commit
>> addresses issues that caused the compiler to emit a warning.
>>
>> This commit is mostly either changing the format string to match the
>> argument, or in some cases, excess, unused arguments are removed.
>> ---
>> sim/ppc/igen.c | 2 +-
>> sim/ppc/ld-cache.c | 4 ++--
>> sim/ppc/ld-decode.c | 2 +-
>> sim/ppc/ld-insn.c | 41 +++++++++++++++--------------------------
>> 4 files changed, 19 insertions(+), 30 deletions(-)
>>
>> diff --git a/sim/ppc/igen.c b/sim/ppc/igen.c
>> index 6a6dbc30f31..27b48638276 100644
>> --- a/sim/ppc/igen.c
>> +++ b/sim/ppc/igen.c
>> @@ -442,7 +442,7 @@ main(int argc,
>> code |= generate_with_icache;
>> break;
>> default:
>> - error (NULL, "Expecting -Ggen-icache or -Ggen-icache=<N>\n");
>> + error ("Expecting -Ggen-icache or -Ggen-icache=<N>\n");
>> }
>> }
>> }
>> diff --git a/sim/ppc/ld-cache.c b/sim/ppc/ld-cache.c
>> index f57f7db650a..b30b1f4d898 100644
>> --- a/sim/ppc/ld-cache.c
>> +++ b/sim/ppc/ld-cache.c
>> @@ -88,13 +88,13 @@ static void
>> dump_cache_rule(cache_table* rule,
>> int indent)
>> {
>> - dumpf(indent, "((cache_table*)0x%x\n", rule);
>> + dumpf(indent, "((cache_table*)0x%p\n", rule);
>
> Replacing "0x%p" with "%p" is recommended (as I submitted earlier).
Thanks. I've made this change locally for now. I'll wait to see if
there's any more feedback over the next couple of days.
Thanks,
Andrew
>
> Thanks,
> Tsukasa
>
>> dumpf(indent, " (type %s)\n", i2name(rule->type, cache_type_map));
>> dumpf(indent, " (field_name \"%s\")\n", rule->field_name);
>> dumpf(indent, " (derived_name \"%s\")\n", rule->derived_name);
>> dumpf(indent, " (type-def \"%s\")\n", rule->type_def);
>> dumpf(indent, " (expression \"%s\")\n", rule->expression);
>> - dumpf(indent, " (next 0x%x)\n", rule->next);
>> + dumpf(indent, " (next 0x%p)\n", rule->next);
>> dumpf(indent, " )\n");
>> }
>>
>> diff --git a/sim/ppc/ld-decode.c b/sim/ppc/ld-decode.c
>> index 68d9f5f4f52..83ff1216b30 100644
>> --- a/sim/ppc/ld-decode.c
>> +++ b/sim/ppc/ld-decode.c
>> @@ -120,7 +120,7 @@ dump_decode_rule(decode_table *rule,
>> dumpf(indent, " (special_mask 0x%x)\n", rule->special_mask);
>> dumpf(indent, " (special_value 0x%x)\n", rule->special_value);
>> dumpf(indent, " (special_constant 0x%x)\n", rule->special_constant);
>> - dumpf(indent, " (next 0x%x)\n", rule->next);
>> + dumpf(indent, " (next 0x%p)\n", rule->next);
>> }
>> dumpf(indent, " )\n");
>> }
>> diff --git a/sim/ppc/ld-insn.c b/sim/ppc/ld-insn.c
>> index 3910af3fdf6..c89c81c1073 100644
>> --- a/sim/ppc/ld-insn.c
>> +++ b/sim/ppc/ld-insn.c
>> @@ -827,29 +827,18 @@ static void
>> dump_insn_field(insn_field *field,
>> int indent)
>> {
>> -
>> - printf("(insn_field*)0x%lx\n", (unsigned long)field);
>> -
>> - dumpf(indent, "(first %d)\n", field->first);
>> -
>> - dumpf(indent, "(last %d)\n", field->last);
>> -
>> - dumpf(indent, "(width %d)\n", field->width);
>> -
>> + printf ("(insn_field*)0x%p\n", field);
>> + dumpf (indent, "(first %d)\n", field->first);
>> + dumpf (indent, "(last %d)\n", field->last);
>> + dumpf (indent, "(width %d)\n", field->width);
>> if (field->is_int)
>> - dumpf(indent, "(is_int %d)\n", field->val_int);
>> -
>> + dumpf (indent, "(is_int %d)\n", field->val_int);
>> if (field->is_slash)
>> - dumpf(indent, "(is_slash)\n");
>> -
>> + dumpf (indent, "(is_slash)\n");
>> if (field->is_string)
>> - dumpf(indent, "(is_string `%s')\n", field->val_string);
>> -
>> - dumpf(indent, "(next 0x%x)\n", field->next);
>> -
>> - dumpf(indent, "(prev 0x%x)\n", field->prev);
>> -
>> -
>> + dumpf (indent, "(is_string `%s')\n", field->val_string);
>> + dumpf (indent, "(next 0x%p)\n", field->next);
>> + dumpf (indent, "(prev 0x%p)\n", field->prev);
>> }
>>
>> static void
>> @@ -860,13 +849,13 @@ dump_insn_fields(insn_fields *fields,
>>
>> printf("(insn_fields*)%p\n", fields);
>>
>> - dumpf(indent, "(first 0x%x)\n", fields->first);
>> - dumpf(indent, "(last 0x%x)\n", fields->last);
>> + dumpf(indent, "(first 0x%p)\n", fields->first);
>> + dumpf(indent, "(last 0x%p)\n", fields->last);
>>
>> dumpf(indent, "(value 0x%x)\n", fields->value);
>>
>> for (i = 0; i < insn_bit_size; i++) {
>> - dumpf(indent, "(bits[%d] ", i, fields->bits[i]);
>> + dumpf(indent, "(bits[%d]", i);
>> dump_insn_field(fields->bits[i], indent+1);
>> dumpf(indent, " )\n");
>> }
>> @@ -961,16 +950,16 @@ dump_insn_table(insn_table *table,
>> dump_opcode_field(table->opcode, indent+1, 1);
>> dumpf(indent, " )\n");
>>
>> - dumpf(indent, "(nr_entries %d)\n", table->entries);
>> + dumpf(indent, "(nr_entries %d)\n", table->nr_entries);
>> dumpf(indent, "(entries ");
>> dump_insn_table(table->entries, indent+1, table->nr_entries);
>> dumpf(indent, " )\n");
>>
>> - dumpf(indent, "(sibling ", table->sibling);
>> + dumpf(indent, "(sibling ");
>> dump_insn_table(table->sibling, indent+1, levels-1);
>> dumpf(indent, " )\n");
>>
>> - dumpf(indent, "(parent ", table->parent);
>> + dumpf(indent, "(parent ");
>> dump_insn_table(table->parent, indent+1, 0);
>> dumpf(indent, " )\n");
>>
On 12 Oct 2022 13:38, Andrew Burgess via Gdb-patches wrote:
> --- a/sim/ppc/igen.c
> +++ b/sim/ppc/igen.c
> @@ -442,7 +442,7 @@ main(int argc,
> code |= generate_with_icache;
> break;
> default:
> - error (NULL, "Expecting -Ggen-icache or -Ggen-icache=<N>\n");
> + error ("Expecting -Ggen-icache or -Ggen-icache=<N>\n");
the file is using inconsistent whitespace now
> --- a/sim/ppc/ld-cache.c
> +++ b/sim/ppc/ld-cache.c
> @@ -88,13 +88,13 @@ static void
> dump_cache_rule(cache_table* rule,
> int indent)
> {
> - dumpf(indent, "((cache_table*)0x%x\n", rule);
> + dumpf(indent, "((cache_table*)0x%p\n", rule);
you don't want 0x with %p as %p already includes it
comes up multiple times in this patch
-mike
Mike Frysinger <vapier@gentoo.org> writes:
> On 12 Oct 2022 13:38, Andrew Burgess via Gdb-patches wrote:
>> --- a/sim/ppc/igen.c
>> +++ b/sim/ppc/igen.c
>> @@ -442,7 +442,7 @@ main(int argc,
>> code |= generate_with_icache;
>> break;
>> default:
>> - error (NULL, "Expecting -Ggen-icache or -Ggen-icache=<N>\n");
>> + error ("Expecting -Ggen-icache or -Ggen-icache=<N>\n");
>
> the file is using inconsistent whitespace now
>
>> --- a/sim/ppc/ld-cache.c
>> +++ b/sim/ppc/ld-cache.c
>> @@ -88,13 +88,13 @@ static void
>> dump_cache_rule(cache_table* rule,
>> int indent)
>> {
>> - dumpf(indent, "((cache_table*)0x%x\n", rule);
>> + dumpf(indent, "((cache_table*)0x%p\n", rule);
>
> you don't want 0x with %p as %p already includes it
>
> comes up multiple times in this patch
Tsukasa OI already pointed this out:
https://sourceware.org/pipermail/gdb-patches/2022-October/192598.html
The version I pushed fixed this error.
Thanks,
Andrew
@@ -442,7 +442,7 @@ main(int argc,
code |= generate_with_icache;
break;
default:
- error (NULL, "Expecting -Ggen-icache or -Ggen-icache=<N>\n");
+ error ("Expecting -Ggen-icache or -Ggen-icache=<N>\n");
}
}
}
@@ -88,13 +88,13 @@ static void
dump_cache_rule(cache_table* rule,
int indent)
{
- dumpf(indent, "((cache_table*)0x%x\n", rule);
+ dumpf(indent, "((cache_table*)0x%p\n", rule);
dumpf(indent, " (type %s)\n", i2name(rule->type, cache_type_map));
dumpf(indent, " (field_name \"%s\")\n", rule->field_name);
dumpf(indent, " (derived_name \"%s\")\n", rule->derived_name);
dumpf(indent, " (type-def \"%s\")\n", rule->type_def);
dumpf(indent, " (expression \"%s\")\n", rule->expression);
- dumpf(indent, " (next 0x%x)\n", rule->next);
+ dumpf(indent, " (next 0x%p)\n", rule->next);
dumpf(indent, " )\n");
}
@@ -120,7 +120,7 @@ dump_decode_rule(decode_table *rule,
dumpf(indent, " (special_mask 0x%x)\n", rule->special_mask);
dumpf(indent, " (special_value 0x%x)\n", rule->special_value);
dumpf(indent, " (special_constant 0x%x)\n", rule->special_constant);
- dumpf(indent, " (next 0x%x)\n", rule->next);
+ dumpf(indent, " (next 0x%p)\n", rule->next);
}
dumpf(indent, " )\n");
}
@@ -827,29 +827,18 @@ static void
dump_insn_field(insn_field *field,
int indent)
{
-
- printf("(insn_field*)0x%lx\n", (unsigned long)field);
-
- dumpf(indent, "(first %d)\n", field->first);
-
- dumpf(indent, "(last %d)\n", field->last);
-
- dumpf(indent, "(width %d)\n", field->width);
-
+ printf ("(insn_field*)0x%p\n", field);
+ dumpf (indent, "(first %d)\n", field->first);
+ dumpf (indent, "(last %d)\n", field->last);
+ dumpf (indent, "(width %d)\n", field->width);
if (field->is_int)
- dumpf(indent, "(is_int %d)\n", field->val_int);
-
+ dumpf (indent, "(is_int %d)\n", field->val_int);
if (field->is_slash)
- dumpf(indent, "(is_slash)\n");
-
+ dumpf (indent, "(is_slash)\n");
if (field->is_string)
- dumpf(indent, "(is_string `%s')\n", field->val_string);
-
- dumpf(indent, "(next 0x%x)\n", field->next);
-
- dumpf(indent, "(prev 0x%x)\n", field->prev);
-
-
+ dumpf (indent, "(is_string `%s')\n", field->val_string);
+ dumpf (indent, "(next 0x%p)\n", field->next);
+ dumpf (indent, "(prev 0x%p)\n", field->prev);
}
static void
@@ -860,13 +849,13 @@ dump_insn_fields(insn_fields *fields,
printf("(insn_fields*)%p\n", fields);
- dumpf(indent, "(first 0x%x)\n", fields->first);
- dumpf(indent, "(last 0x%x)\n", fields->last);
+ dumpf(indent, "(first 0x%p)\n", fields->first);
+ dumpf(indent, "(last 0x%p)\n", fields->last);
dumpf(indent, "(value 0x%x)\n", fields->value);
for (i = 0; i < insn_bit_size; i++) {
- dumpf(indent, "(bits[%d] ", i, fields->bits[i]);
+ dumpf(indent, "(bits[%d]", i);
dump_insn_field(fields->bits[i], indent+1);
dumpf(indent, " )\n");
}
@@ -961,16 +950,16 @@ dump_insn_table(insn_table *table,
dump_opcode_field(table->opcode, indent+1, 1);
dumpf(indent, " )\n");
- dumpf(indent, "(nr_entries %d)\n", table->entries);
+ dumpf(indent, "(nr_entries %d)\n", table->nr_entries);
dumpf(indent, "(entries ");
dump_insn_table(table->entries, indent+1, table->nr_entries);
dumpf(indent, " )\n");
- dumpf(indent, "(sibling ", table->sibling);
+ dumpf(indent, "(sibling ");
dump_insn_table(table->sibling, indent+1, levels-1);
dumpf(indent, " )\n");
- dumpf(indent, "(parent ", table->parent);
+ dumpf(indent, "(parent ");
dump_insn_table(table->parent, indent+1, 0);
dumpf(indent, " )\n");