[PATCH radeontool 5/5] Fix string format mismatch warnings

[Jonathan Nieder]

Tormod Volden wrote:

> Consistently use int for register values and long for register offsets.

Nice idea.

[...]
> --- a/atombios_tables.c
> +++ b/atombios_tables.c
> @@ -634,7 +634,7 @@ static void radeon_print_pp_misc(uint32_t misc)
>      if (misc & ATOM_PM_MISCINFO_THERMAL_DIODE_MODE)
>  	printf("\t\tTHERMAL_DIODE_MODE\n");
>      if (misc & ATOM_PM_MISCINFO_FRAME_MODULATION_MASK)
> -	printf("\t\tFM: 0x%x\n",
> +	printf("\t\tFM: 0x%lx\n",
>  	       (misc & ATOM_PM_MISCINFO_FRAME_MODULATION_MASK) >> ATOM_PM_MISCINFO_FRAME_MODULATION_SHIFT);

Hm, how does this work?

[...]
> --- a/avivotool.c
> +++ b/avivotool.c
> @@ -938,7 +938,7 @@ void eg_cmd_regs(const char *type)
>      int show_all = (strcmp(type, "all") == 0);
>      int show_mc = (show_all || strstr(type, "mc"));
>      int show_grphs = 0;
> -    int i;
> +    unsigned long i;
>      uint32_t tmp, tmp1;
>      char tmpname[10];
>      
> @@ -963,7 +963,7 @@ void eg_cmd_regs(const char *type)
>  	if (tmp & 0x1)
>  	    show_grphs |= (1 << i);
>  
> -	printf("D%dCRTC: %s, Cursor %s\n", i+1, tmp & 0x1 ? "Enabled" : "Disabled",
> +	printf("D%ldCRTC: %s, Cursor %s\n", i+1, tmp & 0x1 ? "Enabled" : "Disabled",
>  	       tmp1 & 0x1 ? "Enabled" : "Disabled");

The format string for "unsigned long" is %lu.

"i" is an array index and is used to number bits and thus can only
be very low.  Why not keep it an "int"?

[...]
> @@ -1052,9 +1052,9 @@ void eg_cmd_regs(const char *type)
>  	    SHOW_UNKNOWN_REG(i);
>  	for (i = 0x6420 ; i < 0x647c; i+=4)
>  	    SHOW_UNKNOWN_REG(i);

Ah, that's why.  I guess the two kinds of index should get different
variables (for example by splitting this into multiple functions so
they get their own scope :)).

> -	SHOW_UNKNOWN_REG(0x6590);
> -	SHOW_UNKNOWN_REG(0x65b4);
> -	SHOW_UNKNOWN_REG(0x65cc);
> +	SHOW_UNKNOWN_REG(0x6590L);
> +	SHOW_UNKNOWN_REG(0x65b4L);
> +	SHOW_UNKNOWN_REG(0x65ccL);

I wonder why this isn't an inline function:

	static inline void SHOW_UNKNOWN_REG(unsigned long offset)
	{
		unsigned int val = radeon_get(offset, "byhand");
		printf("%08lx\t%08x (%d)\n", offset, val, val);
	}

[...]
>  
>  	for (i = 0x6660 ; i <= 0x66dc; i+=4)
>  	    SHOW_UNKNOWN_REG(i);
> @@ -1223,11 +1223,11 @@ void radeon_cmd_regs(const char *type)
>              SHOW_UNKNOWN_REG(i);
>  
>  	/* dump a bunch of the MC regs */
> -#define SHOW_UNK_MC_REG(r) printf("%02x\t%08x\n", r, radeon_get_indexed(AVIVO_MC_INDEX, AVIVO_MC_DATA, (r | 0x007f0000), #r))
> +#define SHOW_UNK_MC_REG(r) printf("%02lx\t%08x\n", r, radeon_get_indexed(AVIVO_MC_INDEX, AVIVO_MC_DATA, (r | 0x007f0000), #r))

This could be an inline function, too:

	static inline void show_unk_mc_reg(unsigned long offset, const char *name)
	{
		...
	}
	#define SHOW_UNK_MC_REG(r) show_unk_mc_reg((r), #r)

Likewise in radeonreg.c.

[...]
> --- a/radeontool.c
> +++ b/radeontool.c
> @@ -849,7 +849,7 @@ void radeon_cmd_bits(void)
>  
>  void radeon_cmd_dac(char *param)
>  {
> -    unsigned long dac_cntl;
> +    unsigned int dac_cntl;
[... etc ...]

Nice.

[...]
> @@ -2379,7 +2379,7 @@ static void radeon_rom_legacy_igptable(unsigned char *bios, int hdr)
>  	fsb = BIOS16(offset + 0x2);
>  	if (rev < 2)
>  	    fsb *= 100;
> -	printf("FSB: %f Mhz\n", fsb);
> +	printf("FSB: %i Mhz\n", fsb);

Good catch, yikes.

Except where noted above,
Reviewed-by: Jonathan Nieder <jrnieder at gmail.com>

Thanks for working on this.