Patchwork linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

login
register
mail settings
Submitter Alexander Graf
Date April 19, 2012, 2:23 p.m.
Message ID <1334845430-24120-1-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/153782/
State New
Headers show

Comments

Alexander Graf - April 19, 2012, 2:23 p.m.
On my PPC host, HOST_LONG_SIZE is not defined even after
running configure. Use the normal C way of determining the
long size instead.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 thunk.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Peter Maydell - April 19, 2012, 2:47 p.m.
On 19 April 2012 15:23, Alexander Graf <agraf@suse.de> wrote:
> On my PPC host, HOST_LONG_SIZE is not defined even after
> running configure. Use the normal C way of determining the
> long size instead.
>  #elif defined(HOST_PPC)
> -            return HOST_LONG_SIZE;
> +            return sizeof(long);
>  #else

HOST_LONG_SIZE was removed by Stefan in commit 9c6ecf;
it looks like this PPC-only use was accidentally omitted.
For consistency with that commit and how we handle TYPE_LONG
in this function, it would be better to use 'sizeof(void *)'
here I think.

-- PMM
Alexander Graf - April 19, 2012, 3:12 p.m.
On 04/19/2012 04:47 PM, Peter Maydell wrote:
> On 19 April 2012 15:23, Alexander Graf<agraf@suse.de>  wrote:
>> On my PPC host, HOST_LONG_SIZE is not defined even after
>> running configure. Use the normal C way of determining the
>> long size instead.
>>   #elif defined(HOST_PPC)
>> -            return HOST_LONG_SIZE;
>> +            return sizeof(long);
>>   #else
> HOST_LONG_SIZE was removed by Stefan in commit 9c6ecf;
> it looks like this PPC-only use was accidentally omitted.
> For consistency with that commit and how we handle TYPE_LONG
> in this function, it would be better to use 'sizeof(void *)'
> here I think.

I'm fairly sure that's not semantically what we're aiming for. We're 
trying to find the host's typdef for the compat_dev_t type.
In fact, that one is defined as u32:

arch/powerpc/include/asm/compat.h:typedef u32        compat_dev_t;

So I'm not sure if sizeof(long) is even correct...
It's certainly more correct than sizeof(void*) and gets us bug 
compatible to where we were before :).


Alex
Peter Maydell - April 19, 2012, 3:55 p.m.
On 19 April 2012 16:12, Alexander Graf <agraf@suse.de> wrote:
> On 04/19/2012 04:47 PM, Peter Maydell wrote:
>> HOST_LONG_SIZE was removed by Stefan in commit 9c6ecf;
>> it looks like this PPC-only use was accidentally omitted.
>> For consistency with that commit and how we handle TYPE_LONG
>> in this function, it would be better to use 'sizeof(void *)'
>> here I think.
>
> I'm fairly sure that's not semantically what we're aiming for. We're trying
> to find the host's typdef for the compat_dev_t type.

Actually I think we want __kernel_old_dev_t: TYPE_OLDDEVT is only used
in the loop_info struct. For PPC that's defined here:
http://lxr.linux.no/#linux+v3.1.1/arch/powerpc/include/asm/posix_types.h#L31
as 'unsigned long' on ppc64 and 'unsigned int' on ppc.
And the way thunk.h finds the size of 'unsigned long' on the
host is by sizeof(void*).

-- PMM
Stefan Weil - April 19, 2012, 7:25 p.m.
Am 19.04.2012 16:47, schrieb Peter Maydell:
> On 19 April 2012 15:23, Alexander Graf <agraf@suse.de> wrote:
>> On my PPC host, HOST_LONG_SIZE is not defined even after
>> running configure. Use the normal C way of determining the
>> long size instead.
>>  #elif defined(HOST_PPC)
>> -            return HOST_LONG_SIZE;
>> +            return sizeof(long);
>>  #else
>
> HOST_LONG_SIZE was removed by Stefan in commit 9c6ecf;
> it looks like this PPC-only use was accidentally omitted.
> For consistency with that commit and how we handle TYPE_LONG
> in this function, it would be better to use 'sizeof(void *)'
> here I think.
>
> -- PMM

The patch which added this HOST_LONG_SIZE was written before
my patch which removed HOST_LONG_SIZE, but it was committed
to QEMU master _after_ my patch.

As Peter wrote, sizeof(void *) would be a good replacement for
PPC(64) here.

Regards,
Stefan W.
Alexander Graf - April 26, 2012, 11:53 a.m.
On 19.04.2012, at 21:25, Stefan Weil wrote:

> Am 19.04.2012 16:47, schrieb Peter Maydell:
>> On 19 April 2012 15:23, Alexander Graf <agraf@suse.de> wrote:
>>> On my PPC host, HOST_LONG_SIZE is not defined even after
>>> running configure. Use the normal C way of determining the
>>> long size instead.
>>> #elif defined(HOST_PPC)
>>> -            return HOST_LONG_SIZE;
>>> +            return sizeof(long);
>>> #else
>> 
>> HOST_LONG_SIZE was removed by Stefan in commit 9c6ecf;
>> it looks like this PPC-only use was accidentally omitted.
>> For consistency with that commit and how we handle TYPE_LONG
>> in this function, it would be better to use 'sizeof(void *)'
>> here I think.
>> 
>> -- PMM
> 
> The patch which added this HOST_LONG_SIZE was written before
> my patch which removed HOST_LONG_SIZE, but it was committed
> to QEMU master _after_ my patch.
> 
> As Peter wrote, sizeof(void *) would be a good replacement for
> PPC(64) here.

Ok, changed to sizeof(void*). Since the patch is so trivial, I won't repost it, but instead just send it out with my next pull request.


Alex
riku.voipio@linaro.org - April 26, 2012, 12:39 p.m.
On 26 April 2012 14:53, Alexander Graf <agraf@suse.de> wrote:
> Ok, changed to sizeof(void*). Since the patch is so trivial, I won't repost it, but instead just send it out with my next pull request.

That's fine from my side.

Riku

Patch

diff --git a/thunk.h b/thunk.h
index 5be8f91..27bb22a 100644
--- a/thunk.h
+++ b/thunk.h
@@ -113,7 +113,7 @@  static inline int thunk_type_size(const argtype *type_ptr, int is_host)
       defined(HOST_PARISC) || defined(HOST_SPARC64)
             return 4;
 #elif defined(HOST_PPC)
-            return HOST_LONG_SIZE;
+            return sizeof(long);
 #else
             return 2;
 #endif