Messages in this thread | | | Date | Thu, 1 Sep 2022 00:04:35 +0300 | From | Dmitry Rokosov <> | Subject | Re: [jic23-iio:testing 124/129] drivers/iio/accel/msa311.c:993:24: warning: format specifies type 'unsigned char' but the argument has type 'unsigned int' |
| |
On Wed, Aug 31, 2022 at 09:38:10AM +0100, Jonathan Cameron wrote: > On Wed, 31 Aug 2022 03:24:05 +0300 > Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote: > > > Hello Jonathan and Andy, > > > > Sorry for such a late response, a couple of days ago my daughter was born. > > So I couldn't reach my laptop :) > > Congratulations and good luck! :)
Thank you! :)
> > > > > >> drivers/iio/accel/msa311.c:993:24: warning: format specifies type 'unsigned char' but the argument has type 'unsigned int' [-Wformat] > > > > > "msa311-%hhx", partid); > > > > > ~~~~ ^~~~~~ > > > > > %x > > > > > 1 warning generated. > > > > > > > > 992 msa311->chip_name = devm_kasprintf(dev, GFP_KERNEL, > > > > > > 993 "msa311-%hhx", partid); > > > > > > > I'm thinking intent here was to limit range of what was printed. Maybe better to use > > > > local u8 variable or cast? > > > > > > > > I can fix it up if that's fine with you - or even better send me a patch that fixes > > > > it however you prefer! > > > > > > Looking back at what Linus said about those specifiers, I would rather > > > go with simple %x or %02x. > > > > > > P.S. Surprisingly many C developers don't know the difference between > > > %hhx and %02x, which is easy to check by > > > > > > char a = -1; > > > printf("%hhx <==> %02x\n", a, a); > > > a = 217; > > > printf("%hhx <==> %02x\n", a, a); > > > > Thank you for pointing to Linus answer. I have explored it at the link: > > > > https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/ > > > > Actually, Linus described one exception to this rule, which I have > > in my patch. I have an integer which I want to print as a char. > > I see that Linus mentions it's a bad idea. I agree with that. But > > currently %hhx => %02x replacement breaks the requested behavior, %02x > > will not shrink integer value to char. I want to say, maybe it's better > > just cast the value to u8 type and print as %x. What do you think? I can > > prepare such a patch. > > > > P.S. Andy's example to show the difference between %hhx and %02x makes > > more clear why such a replacement is not acceptable here. > > > > Output: > > ff <==> ffffffff > > d9 <==> ffffffd9 > > > In this case the storage is an unsigned int, not an unsigned char. > Hence the value will be small and positive. So I'm fairly sure you > won't hit the above because it's > > 0x000000ff --> ff > 0x000000d9 --> d9 > > The range is limited to 8 bits because that's all the underlying register > holds.
From "data" format point of view you are right. We have regmap over I2C and register values will be limited to 8 bits only. But in general unsigned int value bigger than 0xff formatted by %02x will not be limited by two positions only. In other words, we can use a simple %x with the same success. I want to say if our goal is shrinking the unsigned int value to first byte in hex format w/o %hhx using, we need to cast unsigned int value to unsigned char and printout it using simple %x or %02x.
For example, in my opinion, in the below code snippet, only first and third printout formatting are correct. Currently, we are using the second in the merged patchset.
>>> unsigned int a = 0xDEADBEEF; printf("%hhx <==> %02x (uint8_t:%02x)\n", a, a, (unsigned char)a); <<< Output: ef <==> deadbeef (uint8_t:ef) ===
Anyway, regmap over I2C abstraction limits our value to the 8-bit range, so functionally %02x is working well here.
-- Thank you, Dmitry
| |