[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] translate-all: Use proper type
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] translate-all: Use proper type |
Date: |
Tue, 18 Oct 2016 13:43:35 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 10/18/2016 01:34 PM, Emilio G. Cota wrote:
> +++ b/translate-all.c
> @@ -405,23 +405,23 @@ static void page_init(void)
> static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc)
> {
> PageDesc *pd;
> void **lp;
> int i;
>
> /* Level 1. Always allocated. */
> lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
>
> /* Level 2..N-1. */
> for (i = V_L1_SHIFT / V_L2_BITS - 1; i > 0; i--) {
> - void **p = atomic_rcu_read(lp);
> + void *p = atomic_rcu_read(lp);
>
> if (p == NULL) {
> if (!alloc) {
> return NULL;
> }
> p = g_new0(void *, V_L2_SIZE);
> atomic_rcu_set(lp, p);
> }
>
> lp = p + ((index >> (i * V_L2_BITS)) & (V_L2_SIZE - 1));
Pointer addition of 'void *' plus an offset is undefined (gcc, and
presumably clang, have an extension that treats it the same as computing
an offset to a 'char *'; but some compilers choke); this is because
sizeof(void) is unknown, so you don't know what stride to make for each
offset. Or put another way, 'p + offset' is the same as
'&((*p)[offset])', but (*p)[offset] is ill-defined when p is the opaque
type void.
Pointer addition of 'void **' plus an offset is well-defined, because
sizeof(void*) is well-defined and therefore the stride (4 or 8) makes
sense. Or in array notation, computing '&((*p)[offset]) means we are
skipping to the offset array entry where p is the start of the array of
void* pointers.
In that regards, changing the type of p from 'void **' (where you stride
by the size of a pointer when computing lp) to 'void *' (where you
stride by 1 under gcc when computing lp) is WRONG.
> I prefer void **p since that matches lp's and l1_map's type.
Not just prefer, but require.
>
> It's true that since we're dealing with void * the compiler won't
> complain either way.
It's a shame that void* relaxes typing so much, but this is one case
where we HAVE to use the right type.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature