Re: [PATCH] mwifiex: Using %*phD instead of print_hex_dump_bytes

[Joe Perches <joe@perches.com>]

On Thu, 2012-10-04 at 12:16 +0300, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Make output more readable and remove unneeded function call.
> 
> ...
> mwifiex_sdio mmc0:0001:1: last_cmd_index = 3
> last_cmd_id: 00000000: 28 00 28 00 28                            (.(.(
> ...
> 
> would be changed to:
> 
> ...
> mwifiex_sdio mmc0:0001:1: last_cmd_index = 3
> mwifiex_sdio mmc0:0001:1: last_cmd_id: 28 00 28 00 28
> ...

(added Andy Shevchenko)

Hi Andrei.  That's probably a better output style too
as the ascii isn't likely useful.

One non-nit as a style issue for Andy Shevchenko:

> diff --git a/drivers/net/wireless/mwifiex/cmdevt.c b/drivers/net/wireless/mwifiex/cmdevt.c
[]
> @@ -917,21 +917,20 @@ mwifiex_cmd_timeout_func(unsigned long function_context)
[]
> +		dev_err(adapter->dev, "last_cmd_id: %*ph\n", DBG_CMD_NUM,
> +			adapter->dbg.last_cmd_id);
> +		dev_err(adapter->dev, "last_cmd_act: %*ph\n", DBG_CMD_NUM,
> +			adapter->dbg.last_cmd_act);

When the number of bytes is fixed, it might be nicer
to use a format like "%<num>ph" so that the function
argument stack doesn't have the width pushed but it's
fixed in the format.

This case would definitely not work well though as it's
a #define rather than a simple number and I think
using "%" stringize(some_define) "ph" is not nice.

Maybe this case argues more for always using "%*ph".
At least that's easily greppable.

Andy?