[systemd-devel] [PATCH] udev: fix printf specifiers

Shawn Landden shawn at churchofgit.com
Sat Dec 14 21:36:00 PST 2013


On Sat, Dec 14, 2013 at 8:11 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek at in.waw.pl> wrote:
> On Sat, Dec 14, 2013 at 06:48:34PM -0800, Shawn Landden wrote:
>> This keeps the same behavior, which is wierd.
>> ---
>>  src/udev/udev-builtin-path_id.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c
>> index 7476330..7543a11 100644
>> --- a/src/udev/udev-builtin-path_id.c
>> +++ b/src/udev/udev-builtin-path_id.c
>> @@ -71,9 +71,9 @@ static int format_lun_number(struct udev_device *dev, char **path)
>>
>>          /* address method 0, peripheral device addressing with bus id of zero */
>>          if (lun < 256)
>> -                return path_prepend(path, "lun-%d", lun);
>> +                return path_prepend(path, "lun-%hhu", (unsigned char) lun);
>>          /* handle all other lun addressing methods by using a variant of the original lun format */
>> -        return path_prepend(path, "lun-0x%04x%04x00000000", (lun & 0xffff), (lun >> 16) & 0xffff);
>> +        return path_prepend(path, "lun-0x%04hx%04hx00000000", (unsigned short)(lun & 0xffff), (unsigned short)(lun >> 16) & 0xffff);
>>  }
> Hm, nothing wrong with this approach, but I don't like all those casts.
I just decided to be explicit about what the code was doing because it
is confusing, and AFAICT
without any other justification, probably wrong.

it could have been fixed by simply removing the "long" above, as the
top comment says that it is only 32-bits wide.
(and the formatting only supports 32-bits too)

if you are going to put the least siginificant bytes first, wouldn't
you want to do so byte-wise, instead
of creating this mixed-endian monster?
>
> I pushed a simpler change which adds 'l' in appropriate places.
>
> Zbyszek
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel


More information about the systemd-devel mailing list