Convert the remaining calls from newSVpv to newSVpvn as there is no n…

[colinnewell]

…eed for the semantics of newSVpv

I saw a crash in staging from your version of this module, so I'm guessing my initial patches didn't cover all the cases. I've now blanket changed everything as there isn't any code you've got that relies on the rather odd semantics that the newSVpv call has.

[mgregoro]

merged this but having issues with tests failing on builds on armv7 linux with libsodium 1.0.11, and macOS 10.12.2 with the same version of libsodium. Succeeds fine on FreeBSD 10.3 on amd64 though. Going to figure this out before doing another release.

[mgregoro]

Build and all tests succeed with perl 5.18.2 that came with MacOS 10.12.2, but fails after test 3 on my perlbrew built with default settings 5.24.0 on the same machine. I find it strange that the linkers have warnings in both cases, but mostly because macOS's compiler options are trying to build fat binary perl modules and homebrew installed a 64-bit only version of libsodium... I bet it's just how perlbrew is building Perl vs. how Apple built Perl.

perl 5.18.2 (macOS)

/usr/bin/perl /System/Library/Perl/5.18/ExtUtils/xsubpp  -typemap /System/Library/Perl/5.18/ExtUtils/typemap  Sodium.xs > Sodium.xsc && mv Sodium.xsc Sodium.c
cc -c  -I/usr/local/include -I. -arch x86_64 -arch i386 -g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing -fstack-protector -Os   -DVERSION=\"0.10\" -DXS_VERSION=\"0.10\"  "-I/System/Library/Perl/5.18/darwin-thread-multi-2level/CORE"  -Wno-pointer-sign Sodium.c
Running Mkbootstrap for Crypt::Sodium ()
chmod 644 Sodium.bs
rm -f blib/arch/auto/Crypt/Sodium/Sodium.bundle
LD_RUN_PATH="/usr/local/lib" cc -mmacosx-version-min=10.12.2  -arch x86_64 -arch i386 -bundle -undefined dynamic_lookup -fstack-protector Sodium.o  -o blib/arch/auto/Crypt/Sodium/Sodium.bundle    \
       -L/usr/local/lib -lsodium    \

ld: warning: ignoring file /usr/local/lib/libsodium.dylib, file was built for x86_64 which is not the architecture being linked (i386): /usr/local/lib/libsodium.dylib

[mikeyg@skip:~/projects/Crypt-Sodium] master* ± make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/Crypt-Sodium.t .. ok
All tests successful.
Files=1, Tests=16,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.15 cusr  0.03 csys =  0.21 CPU)
Result: PASS

perlbrew 5.24.0

"/Users/mikeyg/perl5/perlbrew/perls/perl-5.24.0/bin/perl" "/Users/mikeyg/perl5/perlbrew/perls/perl-5.24.0/lib/5.24.0/ExtUtils/xsubpp"  -typemap "/Users/mikeyg/perl5/perlbrew/perls/perl-5.24.0/lib/5.24.0/ExtUtils/typemap"  Sodium.xs > Sodium.xsc && mv Sodium.xsc Sodium.c
cc -c  -I/usr/local/include -I. -fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -O3   -DVERSION=\"0.10\" -DXS_VERSION=\"0.10\"  "-I/Users/mikeyg/perl5/perlbrew/perls/perl-5.24.0/lib/5.24.0/darwin-2level/CORE"  -Wno-pointer-sign Sodium.c
rm -f blib/arch/auto/Crypt/Sodium/Sodium.bundle
LD_RUN_PATH="/usr/local/lib" env MACOSX_DEPLOYMENT_TARGET=10.3 cc  -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector-strong Sodium.o  -o blib/arch/auto/Crypt/Sodium/Sodium.bundle   \
       -L/usr/local/lib -lsodium    \

ld: warning: object file (Sodium.o) was built for newer OSX version (10.12) than being linked (10.4)

[mikeyg@skip:~/projects/Crypt-Sodium] master* ± make test
Running Mkbootstrap for Crypt::Sodium ()
chmod 644 "Sodium.bs"
PERL_DL_NONLAZY=1 "/Users/mikeyg/perl5/perlbrew/perls/perl-5.24.0/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/Crypt-Sodium.t .. All 3 subtests passed

Test Summary Report
-------------------
t/Crypt-Sodium.t (Wstat: 6 Tests: 3 Failed: 0)
  Non-zero wait status: 6
  Parse errors: No plan found in TAP output
Files=1, Tests=3,  0 wallclock secs ( 0.01 usr  0.01 sys +  0.01 cusr  0.00 csys =  0.03 CPU)
Result: FAIL
Failed 1/1 test programs. 0/3 subtests failed.
make: *** [test_dynamic] Error 255

[colinnewell]

Perhaps run the tests with PERL5OPT=-d:cst

You'll need Devel::cst installed at the time.

[mgregoro]

I couldn't even get Devel::cst to install. I'm re-exploring my build of 5.24.0 seems perlbrew install perl-5.24.0 doesn't cut it on Sierra.

[mgregoro]

I'm having the exact same problems with Arch Linux's supplied 5.24.0, and also Armbian's 5.22.1 both on armv7. Devel::cst also won't build and test on either. Arch's perl segfaults after successfully completing test 5 (roundtrip of crypto_box), and my perlbrew built perl on MacOS 10.12.2, and Armbian's 5.22.1 both segfault after successfully completing test 3 (roundtrip of '').

Further, this has nothing to do with the switch from newSVpv to newSVpvn, these perls segfault in the same spots using both. So these problems existed before :(.

I'm more concerned about the segfaults on the native perls on arm than I am about my perlbrew 5.24.0. I can't even get 5.24.0 to build on sierra with -Dccflags="-mmacosx-version-min=10.12" -Dccdlflags="-mmacosx-version-min=10.12" -Dldflags="-mmacosx-version-min=10.12" -Dlddlflags="-mmacosx-version-min=10.12" specified, which would have eliminated the only compile time warning I was seeing.

I'm going to open issues for the segfaults on armv7 and since this is building fine on all tested amd64 perls except my weird MacOS build, I'll put together a 0.10 release.

[colinnewell]

Thanks. I'll try to take a look myself at some point on the raspberry pi end.

[mgregoro]

FWIW this is all fixed in the latest 0.10 release which should be hitting CPAN presently. Apparently my use of the assert() macro was disliked on some platforms, probably due to NDEBUG being on or off at their perls' compile time. TIL don't use assert() in perl modules... or plugins of any sort where you don't control compile time of the entire system (duh!).

On both perlbrew installed perls on macOS 10.12.2 it works as well...

perlbrew exec perl Crypt-Sodium.t
perl-5.22.2
==========
ok 1 - use Crypt::Sodium;
ok 2 - Testing roundtrip of crypto_stream_xor
ok 3 - Testing roundtrip of ''
ok 4 - Testing roundtrip of crypto_secretbox
ok 5 - Testing roundtrip of crypto_box
ok 6 - Testing roundtrip of ''
ok 7 - Testing hash comparison
ok 8 - Testing random bytes output
ok 9 - sanity check crypto_hash_scrypt
ok 10 - test crypto_pwhash_scrypt_str_verify positive
ok 11 - test crypto_pwhash_scrypt_str_verify negative
ok 12 - verifying crypto_sign signed message
ok 13 - verifying crypto_sign_detached signature
ok 14 - derive public key from private key using crypto_scalarmult_base
ok 15 - derive shared secret using crypto_scalarmult
ok 16 - derive shared secret using crypto_scalarmult_safe h(q || client_pub || server_pub)
1..16

perl-5.24.0
==========
ok 1 - use Crypt::Sodium;
ok 2 - Testing roundtrip of crypto_stream_xor
ok 3 - Testing roundtrip of ''
ok 4 - Testing roundtrip of crypto_secretbox
ok 5 - Testing roundtrip of crypto_box
ok 6 - Testing roundtrip of ''
ok 7 - Testing hash comparison
ok 8 - Testing random bytes output
ok 9 - sanity check crypto_hash_scrypt
ok 10 - test crypto_pwhash_scrypt_str_verify positive
ok 11 - test crypto_pwhash_scrypt_str_verify negative
ok 12 - verifying crypto_sign signed message
ok 13 - verifying crypto_sign_detached signature
ok 14 - derive public key from private key using crypto_scalarmult_base
ok 15 - derive shared secret using crypto_scalarmult
ok 16 - derive shared secret using crypto_scalarmult_safe h(q || client_pub || server_pub)
1..16

Thank you again for all your help and patience with this, have a great holiday!

[colinnewell]

Excellent, thank you, and the same to you.