develooper Front page | perl.perl5.porters | Postings from May 2013

RE: [perl #117835] Pointer cast to integer of different size in hv_func.h

Thread Previous | Thread Next
From:
Steve Hay
Date:
May 2, 2013 13:30
Subject:
RE: [perl #117835] Pointer cast to integer of different size in hv_func.h
Message ID:
67B2BB40A61BE846B65EF4793B863D6C983EB8@ukmail02.planit.group
Nicholas Clark wrote on 2013-05-02:
> On Thu, May 02, 2013 at 10:33:12PM +1000, sisyphus1@optusnet.com.au
> wrote:
[...]
>> I've tried Robin's patch on 5.17.11 - it's fine for me on Win64. (I
>> haven't tested Win32.)

It works fine for me too, using MinGW 4.7.0 (32-bit native compiler),
MinGW-w64 4.7.1 (32->64-bit cross compiler) and MinGW-w64 4.7.1 (64-bit
native compiler) -- no errors and no warnings.


[...]
>> I also use 4.7.0 for my 32-bit builds. There was no problem there ...
>> but, of course, 'IV' and 'long' are the same thing on 32-bit builds.
> 
> Well, they are by default. The warning is cropping up on *nix when one
> runs Configure with -Duse64bitint on a 32 bit platform, so that IVs
> are long longs.
> 
> 
> Also, Robin's attached *patch* has the 1 line commit message "avoid
> cast from pointer to integer of different size"
> 
> which doesn't reference the RT number (it can't - RT allocated the
> ticket number after submission), and doesn't include the extra
> discussion in the patch. I think the commit message should be
> something more like
> 
> commit 275227021431a7dbfb1c18e88a68a35ce0851161
> Author: Robin Barker <rmbarker@cpan.org>
> Date:   Wed May 1 18:39:26 2013 +0100
> 
>     Use PTR2IV() instead of casting a pointer to IV, to avoid
warnings.
>     
>     The cast to IV was added to avoid problems on platforms where
>     pointers are larger than longs. However, the change instead
>     generates warnings on platforms where IVs are larger than
pointers.
>     Instead, use the PTR2IV() macro provided by perl.h to make
everyone
>     happy.
>     
>     Also change the type of the variable on that line, because it is
>     cast to STRLEN the first time it is used, and then passed to a
macro
>     that assigns it to an int regardless.
> diff --git a/hv_func.h b/hv_func.h
> index 0a66356..9dfe158 100644
> --- a/hv_func.h
> +++ b/hv_func.h
> @@ -392,8 +392,8 @@ S_perl_hash_murmur3(const unsigned char * const
> seed, const
>      /* This CPU does not handle unaligned word access */
>      
>      /* Consume enough so that the next data byte is word aligned */
> -    int i = -(IV)ptr & 3;
> -    if(i && (STRLEN)i <= len) {
> +    STRLEN i = -PTR2IV(ptr) & 3;
> +    if(i && i <= len) {
>        MURMUR_DOBYTES(i, h1, carry, bytes_in_carry, ptr, len);
>      }
> (which I've just pushed to smoke-me/nicholas/rt-117687 In case
George's
> Win32 smoker is able to test this sooner than Steve on Win32)

I've also tested with VC++ 2010 32-bit and 64-bit native compilers. No
errors or warnings from the 32-bit one, but the 64-bit one gets a new
warning:

hv_func.h(397) : warning C4267: 'initializing' : conversion from
'size_t' to 'int', possible loss of data

However, it already has several other warnings from that file:

hv_func.h(264) : warning C4267: 'initializing' : conversion from
'size_t' to 'U32', possible loss of data
hv_func.h(374) : warning C4267: 'initializing' : conversion from
'size_t' to 'I32', possible loss of data
hv_func.h(438) : warning C4267: 'initializing' : conversion from
'size_t' to 'int', possible loss of data
hv_func.h(488) : warning C4267: 'initializing' : conversion from
'size_t' to 'U32', possible loss of data
hv_func.h(503) : warning C4267: 'initializing' : conversion from
'size_t' to 'U32', possible loss of data

and generally gazillions of other warnings throughout the build which
the gcc 64-bit compilers don't do, so I'm inclined to turn a blind eye.

I haven't tried a 32-bit build with 64-bit-ints yet, but I will do later
unless you think it isn't worth the trouble.

PS. I also had Robin's other patch (RT #117837) in place throughout all
this, and that looks fine from here too, although I didn't see the
warnings that it is intended to fix originally anyway.

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About