| Message ID | 1334845430-24120-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
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
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
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
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.
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
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
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
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(-)