[Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

[Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

[Stefan Weil]

This patch adds more printf format checking.

Additional modifications were needed for this code change:

* alpha-dis.c: The local definition of MAX conflicts with
  a previous definition from osdep.h, so add an #undef.

* dis-asm.h: Add include for fprintf_function (qemu-common.h).
  The standard (now redundant) includes are removed.

* mis-dis.c: The definition of ARRAY_SIZE is no longer needed
  and must be removed (conflict with previous definition from
  qemu-common.h).

* sh4-dis.c: Remove some unneeded forward declarations.

Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 alpha-dis.c      |    3 +++
 arm-dis.c        |   14 +++++++-------
 dis-asm.h        |   10 ++--------
 m68k-dis.c       |    2 +-
 microblaze-dis.c |    2 +-
 mips-dis.c       |    2 --
 sh4-dis.c        |   16 +++++-----------
 7 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/alpha-dis.c b/alpha-dis.c
index 970da5b..8a2411e 100644
--- a/alpha-dis.c
+++ b/alpha-dis.c
@@ -22,6 +22,9 @@ along with this file; see the file COPYING.  If not, see
 #include <stdio.h>
 #include "dis-asm.h"
 
+/* MAX is redefined below, so remove any previous definition. */
+#undef MAX
+
 /* The opcode table is an array of struct alpha_opcode.  */
 
 struct alpha_opcode
diff --git a/arm-dis.c b/arm-dis.c
index fe7ac99..af21739 100644
--- a/arm-dis.c
+++ b/arm-dis.c
@@ -1587,7 +1587,7 @@ arm_decode_bitfield (const char *ptr, unsigned long insn,
 }
 
 static void
-arm_decode_shift (long given, fprintf_ftype func, void *stream,
+arm_decode_shift (long given, fprintf_function func, void *stream,
 		  int print_shift)
 {
   func (stream, "%s", arm_regnames[given & 0xf]);
@@ -1633,7 +1633,7 @@ print_insn_coprocessor (bfd_vma pc, struct disassemble_info *info, long given,
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
   unsigned long mask;
   unsigned long value;
   int cond;
@@ -2127,7 +2127,7 @@ static void
 print_arm_address (bfd_vma pc, struct disassemble_info *info, long given)
 {
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (((given & 0x000f0000) == 0x000f0000)
       && ((given & 0x02000000) == 0))
@@ -2222,7 +2222,7 @@ print_insn_neon (struct disassemble_info *info, long given, bfd_boolean thumb)
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (thumb)
     {
@@ -2676,7 +2676,7 @@ print_insn_arm_internal (bfd_vma pc, struct disassemble_info *info, long given)
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (print_insn_coprocessor (pc, info, given, false))
     return;
@@ -3036,7 +3036,7 @@ print_insn_thumb16 (bfd_vma pc, struct disassemble_info *info, long given)
 {
   const struct opcode16 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   for (insn = thumb_opcodes; insn->assembler; insn++)
     if ((given & insn->mask) == insn->value)
@@ -3312,7 +3312,7 @@ print_insn_thumb32 (bfd_vma pc, struct disassemble_info *info, long given)
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (print_insn_coprocessor (pc, info, given, true))
     return;
diff --git a/dis-asm.h b/dis-asm.h
index 9b9657e..3fb4838 100644
--- a/dis-asm.h
+++ b/dis-asm.h
@@ -9,11 +9,7 @@
 #ifndef DIS_ASM_H
 #define DIS_ASM_H
 
-#include <stdlib.h>
-#include <stdbool.h>
-#include <stdio.h>
-#include <string.h>
-#include <inttypes.h>
+#include "qemu-common.h"
 
 typedef void *PTR;
 typedef uint64_t bfd_vma;
@@ -237,8 +233,6 @@ typedef struct symbol_cache_entry
     } udata;
 } asymbol;
 
-typedef int (*fprintf_ftype) (FILE*, const char*, ...);
-
 enum dis_insn_type {
   dis_noninsn,			/* Not a valid instruction */
   dis_nonbranch,		/* Not a branch instruction */
@@ -261,7 +255,7 @@ enum dis_insn_type {
    by hand, or using one of the initialization macros below.  */
 
 typedef struct disassemble_info {
-  fprintf_ftype fprintf_func;
+  fprintf_function fprintf_func;
   FILE *stream;
   PTR application_data;
 
diff --git a/m68k-dis.c b/m68k-dis.c
index d93943e..04f837a 100644
--- a/m68k-dis.c
+++ b/m68k-dis.c
@@ -1732,7 +1732,7 @@ match_insn_m68k (bfd_vma memaddr,
   const char *d;
 
   bfd_byte *buffer = priv->the_buffer;
-  fprintf_ftype save_printer = info->fprintf_func;
+  fprintf_function save_printer = info->fprintf_func;
   void (* save_print_address) (bfd_vma, struct disassemble_info *)
     = info->print_address_func;
 
diff --git a/microblaze-dis.c b/microblaze-dis.c
index 7694a43..16c312f 100644
--- a/microblaze-dis.c
+++ b/microblaze-dis.c
@@ -789,7 +789,7 @@ read_insn_microblaze (bfd_vma memaddr,
 int 
 print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
 {
-  fprintf_ftype       fprintf_func = info->fprintf_func;
+  fprintf_function    fprintf_func = info->fprintf_func;
   void *              stream = info->stream;
   unsigned long       inst, prev_inst;
   struct op_code_struct * op, *pop;
diff --git a/mips-dis.c b/mips-dis.c
index 279b591..4d8e85b 100644
--- a/mips-dis.c
+++ b/mips-dis.c
@@ -3117,8 +3117,6 @@ struct mips_arch_choice
 #define bfd_mach_mipsisa64             64
 #define bfd_mach_mipsisa64r2           65
 
-#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
-
 static const struct mips_arch_choice mips_arch_choices[] =
 {
   { "numeric",	0, 0, 0, 0,
diff --git a/sh4-dis.c b/sh4-dis.c
index 078a6b2..673bc78 100644
--- a/sh4-dis.c
+++ b/sh4-dis.c
@@ -1163,15 +1163,9 @@ const sh_opcode_info sh_table[] =
 #define INCLUDE_SHMEDIA
 #endif
 
-static void print_movxy
-  (const sh_opcode_info *, int, int, fprintf_ftype, void *);
-static void print_insn_ddt (int, struct disassemble_info *);
-static void print_dsp_reg (int, fprintf_ftype, void *);
-static void print_insn_ppi (int, struct disassemble_info *);
-
 static void
 print_movxy (const sh_opcode_info *op, int rn, int rm,
-	     fprintf_ftype fprintf_fn, void *stream)
+             fprintf_function fprintf_fn, void *stream)
 {
   int n;
 
@@ -1247,7 +1241,7 @@ print_movxy (const sh_opcode_info *op, int rn, int rm,
 static void
 print_insn_ddt (int insn, struct disassemble_info *info)
 {
-  fprintf_ftype fprintf_fn = info->fprintf_func;
+  fprintf_function fprintf_fn = info->fprintf_func;
   void *stream = info->stream;
 
   /* If this is just a nop, make sure to emit something.  */
@@ -1332,7 +1326,7 @@ print_insn_ddt (int insn, struct disassemble_info *info)
 }
 
 static void
-print_dsp_reg (int rm, fprintf_ftype fprintf_fn, void *stream)
+print_dsp_reg (int rm, fprintf_function fprintf_fn, void *stream)
 {
   switch (rm)
     {
@@ -1377,7 +1371,7 @@ print_insn_ppi (int field_b, struct disassemble_info *info)
 {
   static const char *sx_tab[] = { "x0", "x1", "a0", "a1" };
   static const char *sy_tab[] = { "y0", "y1", "m0", "m1" };
-  fprintf_ftype fprintf_fn = info->fprintf_func;
+  fprintf_function fprintf_fn = info->fprintf_func;
   void *stream = info->stream;
   unsigned int nib1, nib2, nib3;
   unsigned int altnib1, nib4;
@@ -1520,7 +1514,7 @@ print_insn_ppi (int field_b, struct disassemble_info *info)
 int
 print_insn_sh (bfd_vma memaddr, struct disassemble_info *info)
 {
-  fprintf_ftype fprintf_fn = info->fprintf_func;
+  fprintf_function fprintf_fn = info->fprintf_func;
   void *stream = info->stream;
   unsigned char insn[4];
   unsigned char nibs[8];
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

[Andreas Färber]

Am 15.11.2010 um 19:39 schrieb Stefan Weil:

> This patch adds more printf format checking.
>
> Additional modifications were needed for this code change:
>
> * alpha-dis.c: The local definition of MAX conflicts with
>  a previous definition from osdep.h, so add an #undef.
>
> * dis-asm.h: Add include for fprintf_function (qemu-common.h).
>  The standard (now redundant) includes are removed.
>
> * mis-dis.c:

:-)

[...]
> alpha-dis.c      |    3 +++
> arm-dis.c        |   14 +++++++-------
> dis-asm.h        |   10 ++--------
> m68k-dis.c       |    2 +-
> microblaze-dis.c |    2 +-
> mips-dis.c       |    2 --
> sh4-dis.c        |   16 +++++-----------
> 7 files changed, 19 insertions(+), 30 deletions(-)

Re: [Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

[Stefan Weil]

Am 15.11.2010 21:01, schrieb Andreas Färber:
> Am 15.11.2010 um 19:39 schrieb Stefan Weil:
>
>> This patch adds more printf format checking.
>>
>> Additional modifications were needed for this code change:
>>
>> * alpha-dis.c: The local definition of MAX conflicts with
>>  a previous definition from osdep.h, so add an #undef.
>>
>> * dis-asm.h: Add include for fprintf_function (qemu-common.h).
>>  The standard (now redundant) includes are removed.
>>
>> * mis-dis.c:
>
> :-)

Thanks. Hopefully nobody will mis-interpret the missing 'p' :-)

Maybe Blue Swirl can add it, or simply commit it as it is
(so future readers of the commit log have fun, too).

[Qemu-devel] Re: [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

[Blue Swirl]

Thanks, applied.

On Mon, Nov 15, 2010 at 6:39 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> This patch adds more printf format checking.
>
> Additional modifications were needed for this code change:
>
> * alpha-dis.c: The local definition of MAX conflicts with
>  a previous definition from osdep.h, so add an #undef.
>
> * dis-asm.h: Add include for fprintf_function (qemu-common.h).
>  The standard (now redundant) includes are removed.
>
> * mis-dis.c: The definition of ARRAY_SIZE is no longer needed
>  and must be removed (conflict with previous definition from
>  qemu-common.h).
>
> * sh4-dis.c: Remove some unneeded forward declarations.
>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  alpha-dis.c      |    3 +++
>  arm-dis.c        |   14 +++++++-------
>  dis-asm.h        |   10 ++--------
>  m68k-dis.c       |    2 +-
>  microblaze-dis.c |    2 +-
>  mips-dis.c       |    2 --
>  sh4-dis.c        |   16 +++++-----------
>  7 files changed, 19 insertions(+), 30 deletions(-)
>
> diff --git a/alpha-dis.c b/alpha-dis.c
> index 970da5b..8a2411e 100644
> --- a/alpha-dis.c
> +++ b/alpha-dis.c
> @@ -22,6 +22,9 @@ along with this file; see the file COPYING.  If not, see
>  #include <stdio.h>
>  #include "dis-asm.h"
>
> +/* MAX is redefined below, so remove any previous definition. */
> +#undef MAX
> +
>  /* The opcode table is an array of struct alpha_opcode.  */
>
>  struct alpha_opcode
> diff --git a/arm-dis.c b/arm-dis.c
> index fe7ac99..af21739 100644
> --- a/arm-dis.c
> +++ b/arm-dis.c
> @@ -1587,7 +1587,7 @@ arm_decode_bitfield (const char *ptr, unsigned long insn,
>  }
>
>  static void
> -arm_decode_shift (long given, fprintf_ftype func, void *stream,
> +arm_decode_shift (long given, fprintf_function func, void *stream,
>                  int print_shift)
>  {
>   func (stream, "%s", arm_regnames[given & 0xf]);
> @@ -1633,7 +1633,7 @@ print_insn_coprocessor (bfd_vma pc, struct disassemble_info *info, long given,
>  {
>   const struct opcode32 *insn;
>   void *stream = info->stream;
> -  fprintf_ftype func = info->fprintf_func;
> +  fprintf_function func = info->fprintf_func;
>   unsigned long mask;
>   unsigned long value;
>   int cond;
> @@ -2127,7 +2127,7 @@ static void
>  print_arm_address (bfd_vma pc, struct disassemble_info *info, long given)
>  {
>   void *stream = info->stream;
> -  fprintf_ftype func = info->fprintf_func;
> +  fprintf_function func = info->fprintf_func;
>
>   if (((given & 0x000f0000) == 0x000f0000)
>       && ((given & 0x02000000) == 0))
> @@ -2222,7 +2222,7 @@ print_insn_neon (struct disassemble_info *info, long given, bfd_boolean thumb)
>  {
>   const struct opcode32 *insn;
>   void *stream = info->stream;
> -  fprintf_ftype func = info->fprintf_func;
> +  fprintf_function func = info->fprintf_func;
>
>   if (thumb)
>     {
> @@ -2676,7 +2676,7 @@ print_insn_arm_internal (bfd_vma pc, struct disassemble_info *info, long given)
>  {
>   const struct opcode32 *insn;
>   void *stream = info->stream;
> -  fprintf_ftype func = info->fprintf_func;
> +  fprintf_function func = info->fprintf_func;
>
>   if (print_insn_coprocessor (pc, info, given, false))
>     return;
> @@ -3036,7 +3036,7 @@ print_insn_thumb16 (bfd_vma pc, struct disassemble_info *info, long given)
>  {
>   const struct opcode16 *insn;
>   void *stream = info->stream;
> -  fprintf_ftype func = info->fprintf_func;
> +  fprintf_function func = info->fprintf_func;
>
>   for (insn = thumb_opcodes; insn->assembler; insn++)
>     if ((given & insn->mask) == insn->value)
> @@ -3312,7 +3312,7 @@ print_insn_thumb32 (bfd_vma pc, struct disassemble_info *info, long given)
>  {
>   const struct opcode32 *insn;
>   void *stream = info->stream;
> -  fprintf_ftype func = info->fprintf_func;
> +  fprintf_function func = info->fprintf_func;
>
>   if (print_insn_coprocessor (pc, info, given, true))
>     return;
> diff --git a/dis-asm.h b/dis-asm.h
> index 9b9657e..3fb4838 100644
> --- a/dis-asm.h
> +++ b/dis-asm.h
> @@ -9,11 +9,7 @@
>  #ifndef DIS_ASM_H
>  #define DIS_ASM_H
>
> -#include <stdlib.h>
> -#include <stdbool.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <inttypes.h>
> +#include "qemu-common.h"
>
>  typedef void *PTR;
>  typedef uint64_t bfd_vma;
> @@ -237,8 +233,6 @@ typedef struct symbol_cache_entry
>     } udata;
>  } asymbol;
>
> -typedef int (*fprintf_ftype) (FILE*, const char*, ...);
> -
>  enum dis_insn_type {
>   dis_noninsn,                 /* Not a valid instruction */
>   dis_nonbranch,               /* Not a branch instruction */
> @@ -261,7 +255,7 @@ enum dis_insn_type {
>    by hand, or using one of the initialization macros below.  */
>
>  typedef struct disassemble_info {
> -  fprintf_ftype fprintf_func;
> +  fprintf_function fprintf_func;
>   FILE *stream;
>   PTR application_data;
>
> diff --git a/m68k-dis.c b/m68k-dis.c
> index d93943e..04f837a 100644
> --- a/m68k-dis.c
> +++ b/m68k-dis.c
> @@ -1732,7 +1732,7 @@ match_insn_m68k (bfd_vma memaddr,
>   const char *d;
>
>   bfd_byte *buffer = priv->the_buffer;
> -  fprintf_ftype save_printer = info->fprintf_func;
> +  fprintf_function save_printer = info->fprintf_func;
>   void (* save_print_address) (bfd_vma, struct disassemble_info *)
>     = info->print_address_func;
>
> diff --git a/microblaze-dis.c b/microblaze-dis.c
> index 7694a43..16c312f 100644
> --- a/microblaze-dis.c
> +++ b/microblaze-dis.c
> @@ -789,7 +789,7 @@ read_insn_microblaze (bfd_vma memaddr,
>  int
>  print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
>  {
> -  fprintf_ftype       fprintf_func = info->fprintf_func;
> +  fprintf_function    fprintf_func = info->fprintf_func;
>   void *              stream = info->stream;
>   unsigned long       inst, prev_inst;
>   struct op_code_struct * op, *pop;
> diff --git a/mips-dis.c b/mips-dis.c
> index 279b591..4d8e85b 100644
> --- a/mips-dis.c
> +++ b/mips-dis.c
> @@ -3117,8 +3117,6 @@ struct mips_arch_choice
>  #define bfd_mach_mipsisa64             64
>  #define bfd_mach_mipsisa64r2           65
>
> -#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
> -
>  static const struct mips_arch_choice mips_arch_choices[] =
>  {
>   { "numeric", 0, 0, 0, 0,
> diff --git a/sh4-dis.c b/sh4-dis.c
> index 078a6b2..673bc78 100644
> --- a/sh4-dis.c
> +++ b/sh4-dis.c
> @@ -1163,15 +1163,9 @@ const sh_opcode_info sh_table[] =
>  #define INCLUDE_SHMEDIA
>  #endif
>
> -static void print_movxy
> -  (const sh_opcode_info *, int, int, fprintf_ftype, void *);
> -static void print_insn_ddt (int, struct disassemble_info *);
> -static void print_dsp_reg (int, fprintf_ftype, void *);
> -static void print_insn_ppi (int, struct disassemble_info *);
> -
>  static void
>  print_movxy (const sh_opcode_info *op, int rn, int rm,
> -            fprintf_ftype fprintf_fn, void *stream)
> +             fprintf_function fprintf_fn, void *stream)
>  {
>   int n;
>
> @@ -1247,7 +1241,7 @@ print_movxy (const sh_opcode_info *op, int rn, int rm,
>  static void
>  print_insn_ddt (int insn, struct disassemble_info *info)
>  {
> -  fprintf_ftype fprintf_fn = info->fprintf_func;
> +  fprintf_function fprintf_fn = info->fprintf_func;
>   void *stream = info->stream;
>
>   /* If this is just a nop, make sure to emit something.  */
> @@ -1332,7 +1326,7 @@ print_insn_ddt (int insn, struct disassemble_info *info)
>  }
>
>  static void
> -print_dsp_reg (int rm, fprintf_ftype fprintf_fn, void *stream)
> +print_dsp_reg (int rm, fprintf_function fprintf_fn, void *stream)
>  {
>   switch (rm)
>     {
> @@ -1377,7 +1371,7 @@ print_insn_ppi (int field_b, struct disassemble_info *info)
>  {
>   static const char *sx_tab[] = { "x0", "x1", "a0", "a1" };
>   static const char *sy_tab[] = { "y0", "y1", "m0", "m1" };
> -  fprintf_ftype fprintf_fn = info->fprintf_func;
> +  fprintf_function fprintf_fn = info->fprintf_func;
>   void *stream = info->stream;
>   unsigned int nib1, nib2, nib3;
>   unsigned int altnib1, nib4;
> @@ -1520,7 +1514,7 @@ print_insn_ppi (int field_b, struct disassemble_info *info)
>  int
>  print_insn_sh (bfd_vma memaddr, struct disassemble_info *info)
>  {
> -  fprintf_ftype fprintf_fn = info->fprintf_func;
> +  fprintf_function fprintf_fn = info->fprintf_func;
>   void *stream = info->stream;
>   unsigned char insn[4];
>   unsigned char nibs[8];
> --
> 1.7.2.3
>
>