[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/8] 9pfs: fix P9_NOTAG and P9_NOFID macros
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 3/8] 9pfs: fix P9_NOTAG and P9_NOFID macros |
Date: |
Sat, 10 Dec 2016 10:24:35 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
On 12/10/2016 07:57 AM, Greg Kurz wrote:
>>> -#define P9_NOTAG (u16)(~0)
>>> -#define P9_NOFID (u32)(~0)
>>> +#define P9_NOTAG (uint16_t)(~0)
>>> +#define P9_NOFID (uint32_t)(~0)
>>
>> Don't you want to write ((uint16_t)(~0)), to ensure that this expression
>> can be used as a drop-in in any other syntactical situation?
>>
>
> These defines come from the linux kernel sources and I must admit it
> didn't cross my mind... can you share a case where this would cause
> troubles ?
Unlikely to occur in real code, but:
int a[] = { -2, -3 };
int *b = a + 1;
printf("%d\n", (uint16_t)(~0)[b]); // prints 65534 - let's see why?
// prints 65534, or the result of b[-1] cast to uint16_t
printf("%d\n", (uint16_t)((~0)[b]));
// probably dumps core, as b[65535] is out of bounds
printf("%d\n", ((uint16_t)(~0))[b]);
that is, since [] has higher precedence than casts, failure to
parenthesize a cast will change the interpretation of P9_NOTAG[pointer].
And yes, if you copied from the kernel, that means the kernel has a bug
(even if it is unlikely to trip up normal code).
>
>> Or even write it as UINT16_C(~0) (using <stdint.h>), or as UINT16_MAX.
>> (Be aware: the type of (uint16_t)(~0) is uint16_t, while the type of
>> UINT16_MAX is int, due to the rules of integer promotion, if that matters)
>>
>
> UINT16_C(~0) expands to ~0 and UINT16_MAX expands to (65535), at least on
> my laptop (glibc-headers-2.23.1-11.fc24.x86_64)... doesn't that mean the
> type of UINT16_C(~0) is also int ? Please enlighten me.
Indeed, UINT16_C produces an int constant, not uint16_t (since there is
no such thing as a uint16_t constant). So the cast is the only way to
force ~0 to be truncated to a 16-bit pattern. But using UINT16_MAX is
probably just fine, as it is the all-ones value with the correct integer
promotion for use in any other arithmetic.
>
> The 9P spec at http://man.cat-v.org/plan_9/5/version says "(ushort)~0". My
> understanding is 16 bits all ones. I guess I'd rather then go for
> ((uint16_t)(~0)).
Verbose, but works, as does UINT16_MAX. But I stand corrected that
UINT16_C(~0) does not work.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 4/8] tests: virtio-9p: add version operation test, Greg Kurz, 2016/12/09
[Qemu-devel] [PATCH 5/8] tests: virtio-9p: add attach operation test, Greg Kurz, 2016/12/09
[Qemu-devel] [PATCH 6/8] tests: virtio-9p: add walk operation test, Greg Kurz, 2016/12/09
[Qemu-devel] [PATCH 7/8] tests: virtio-9p: no slash in path elements during walk, Greg Kurz, 2016/12/09
[Qemu-devel] [PATCH 8/8] tests: virtio-9p: ".." cannot be used to walk out of the shared directory, Greg Kurz, 2016/12/09