On Wed, May 7, 2014 at 5:53 AM, Dave Mitchell <[email protected]> wrote:
> On Tue, May 06, 2014 at 07:58:41PM -0500, Craig A. Berry wrote:
>> On Tue, May 6, 2014 at 6:01 PM, Steve Hay <[email protected]> wrote:
>> > On 6 May 2014 21:58, "George Greer" <[email protected]> wrote:
>> >>
>> >> Smoke logs available at
>> >> http://m-l.org/~perl/smoke/perl/linux/blead_g++_quick/logdc396cc29397b262d3cc1473ade4229c84e82ca3.log.gz
>> >>
>> >> Automated smoke report for 5.19.12 patch
>> >> dc396cc29397b262d3cc1473ade4229c84e82ca3 v5.19.11-35-gdc396cc
>> > [...]
>> >
>> >> Byte.c: In function ‘void Encode_XSEncoding(encode_t*)’:
>> >> Byte.c:24:25: error: invalid conversion from ‘const char*’ to ‘char*’
>> >> [-fpermissive]
>> >
>> > Sorry folks, looks like upgrading Encode has broken the build again. Good
>> > CPAN Testers results seem not to be enough; I should have tested with a
>> > smoke-me branch first...
>> >
>> > Everything built and tested fine for me. Does anyone that can reproduce 
>> > this
>> > have any spare tuits to fix it? If not then I'll roll back the upgrade.
>> > (We'll end up with blead customizations either way until another new Encode
>> > arrives.)
>>
>> The tuits are hard to come by, but it's readily reproducible by
>> configuring with:
>>
>> $ ./Configure -Dusedevel -Dcc=g++ -des
>>
>> using
>>
>> $ g++ --version
>> g++ (GCC) 4.8.2
>> Copyright (C) 2013 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
>> This on OS X Mavericks but the platform probably doesn't matter.  The
>> error with a wee bit more context is:
>>
>> g++ -c  -I../Encode -fno-common -DPERL_DARWIN -fwrapv
>> -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include
>> -I/opt/local/include -O3   -DVERSION=\"2.04\" -DXS_VERSION=\"2.04\"
>> "-I../../.."   Byte.c
>> Byte.c: In function 'void Encode_XSEncoding(encode_t*)':
>> Byte.c:24:25: error: invalid conversion from 'const char*' to 'char*'
>> [-fpermissive]
>>   SvPVX(iv) = enc->name[0];
>
>
> The new change seems to be fundamentally Broken. The diff (one of several
> similar ones) is:
>
> - SV *sv    = sv_bless(newRV_noinc(newSViv(PTR2IV(enc))),stash);
> + SV *iv    = newSViv(PTR2IV(enc));
> + SV *sv    = sv_bless(newRV_noinc(iv),stash);
>   int i = 0;
> + SvFLAGS(iv) |= SVp_POK;
> + SvPVX(iv) = enc->name[0];
>
> Unless I'm misreading that, it's creating a new SVt_IV, then setting the
> SVp_POK flag on it and assigning to SvPVX, all without first upgrading to
> SvPVIV first (nor setting SvLEN etc).
>
> Also, since its only an SVt_IV, the IV slot is shared with the PV slot,
> so the IV value will be overwritten.
>
> Since I don't know what the intent of this change is, I don't know how to
> fix it.

I thought it looked weird too.  The commit that introduced that code is at:

https://github.com/dankogai/p5-encode/commit/f015cd2a22823d32bf30a1af1de9dbe8879b63d0

which is a revision of:

https://github.com/rurban/p5-encode/commit/1cb0525901f51aab58adb401935039db667602fc

Reply via email to