Message ID | 1270765360-5008-1-git-send-email-thomas_ml@monjalon.net |
---|---|
State | New |
Headers | show |
> { > - fprintf_ftype fprintf = info->fprintf_func; > void * stream = info->stream; I'm pretty sure this is not the correct fix. "Fix a warning" is not sufficient justification for any change. We need to understand what was wrong with the old code, and why the new code is correct. Paul
On 04/09/2010 12:22 AM, Thomas Monjalon wrote: > From: Thomas Monjalon<thomas@monjalon.net> > > Using GCC-4.2.4-1ubuntu4, there were 3 warnings. The last two are correct, but what was the first error? If it was a shadowed declaration as it seems to be, the solution is to s/fprintf/fprintf_func/ throughout print_insn_microblaze (for example). Paolo
Paolo Bonzini wrote: > On 04/09/2010 12:22 AM, Thomas Monjalon wrote: > > Using GCC-4.2.4-1ubuntu4, there were 3 warnings. > > The last two are correct, but what was the first error? If it was a > shadowed declaration as it seems to be, the solution is to > s/fprintf/fprintf_func/ throughout print_insn_microblaze (for example). Paul, Paolo, you're right. Nice review ! The error message is: microblaze-dis.c:792: warning: unused variable 'fprintf' But the error message is bogus. It is a shadowed declaration. By adding -Wshadow, the message is: microblaze-dis.c:792: warning: declaration of 'fprintf' shadows a global declaration /usr/include/stdio.h:332: warning: shadowed declaration is here microblaze-dis.c:792: warning: unused variable 'fprintf' Since the function fprintf is used, removing this declaration is a wrong fix. I am going to send a new patch which renames the variable to fprintf_func as Paolo suggests. Thanks
On 04/09/2010 05:31 PM, Thomas Monjalon wrote: > Paolo Bonzini wrote: >> On 04/09/2010 12:22 AM, Thomas Monjalon wrote: >>> Using GCC-4.2.4-1ubuntu4, there were 3 warnings. >> >> The last two are correct, but what was the first error? If it was a >> shadowed declaration as it seems to be, the solution is to >> s/fprintf/fprintf_func/ throughout print_insn_microblaze (for example). > > Paul, Paolo, you're right. > Nice review ! > > The error message is: > > microblaze-dis.c:792: warning: unused variable 'fprintf' > > But the error message is bogus. It is a shadowed declaration. ... which is hidden because FORTIFY_SOURCE changes fprintf to fprintf_chk. So you're fixing a bug too! :-) Paolo
diff --git a/microblaze-dis.c b/microblaze-dis.c index b26572f..698ea7b 100644 --- a/microblaze-dis.c +++ b/microblaze-dis.c @@ -789,7 +789,6 @@ read_insn_microblaze (bfd_vma memaddr, int print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info) { - fprintf_ftype fprintf = info->fprintf_func; void * stream = info->stream; unsigned long inst, prev_inst; struct op_code_struct * op, *pop; @@ -826,7 +825,7 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info) prev_insn_vma = curr_insn_vma; if (op->name == 0) { - fprintf (stream, ".short 0x%04x", inst); + fprintf (stream, ".short 0x%04lx", inst); } else { @@ -959,7 +958,7 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info) break; default: /* if the disassembler lags the instruction set */ - fprintf (stream, "\tundecoded operands, inst is 0x%04x", inst); + fprintf (stream, "\tundecoded operands, inst is 0x%04lx", inst); break; } }