On Fri, 2012-10-05 at 09:53 +0300, Andy Shevchenko wrote: > On Thu, 2012-10-04 at 09:32 -0700, Joe Perches wrote: > > On Thu, 2012-10-04 at 12:16 +0300, Andrei Emeltchenko wrote: > > > From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx> > > > > > > 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? > First of all, the current linux-next has at least two definitions of > that constant: > drivers/net/wireless/mwifiex/ioctl.h:175:#define DBG_CMD_NUM 5 > drivers/net/wireless/mwifiex/main.h:118:#define DBG_CMD_NUM 5 > > I checked briefly the code and found that the constant is heavily used > as a definition for the length of static byte arrays. So, in that case > probably better to use %*ph, but apply sizeof(cool_var) instead of (int)sizeof(cool_var) of course > putting constant there. And one finding more: buffers are defined as u16. I'm afraid the both previous and proposed versions are printing something interesting, like only half of the defined data. -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
