[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Chicken-hackers] this code looks wrong to me
From: |
Peter Bex |
Subject: |
Re: [Chicken-hackers] this code looks wrong to me |
Date: |
Thu, 18 Feb 2016 22:05:04 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Feb 17, 2016 at 01:53:00PM +0100, Jörg F. Wittenberger wrote:
> Stupid me.
>
> The other way around attached.
Thanks. This patch looked good to me, but I had a bad feeling that this
could go so wrong without being detected, so I wrote some tests.
Then I found that the whole thing is completely broken! You can't use
C_peek_unsigned_integer by re-wrapping a direct pointer like that,
because it will use the stored object's data pointer as a C_word *,
which in the s32 case on a 64-bit machine means the s32 * will get cast
to a s64 *, causing wrong values to be returned and possibly segfaulting.
Attached is a proper patch which thankfully is shorter and simpler:
it simply casts the pointer to the correct type and converts the result
to a fixnum or flonum (or bignum on CHICKEN 5).
I simplified the f32 and f64 locative returning code to re-use the same
buffer that we use for the u32 and s32 (s64, u64) case. We should
probably get rid of the ___tmpflonum stuff in other code too; it's
unnecessarily tricky and can be done a bit simpler. Besides,
WORDS_PER_FLONUM is defined in runtime.c while C_alloc_flonum and
C_kontinue_flonum are defined in chicken.h; this is kind of bogus.
Anyway, something for later!
Cheers,
Peter
0001-Fix-references-into-u32-and-s32-locatives.chicken-5.patch
Description: Text Data
0001-Fix-references-into-u32-and-s32-locatives.master.patch
Description: Text Data
signature.asc
Description: Digital signature