Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"sv_bless( self, gv_stashpv( class, 0 ) )" issue #10

Open
bulk88 opened this issue Jul 2, 2014 · 6 comments
Open

"sv_bless( self, gv_stashpv( class, 0 ) )" issue #10

bulk88 opened this issue Jul 2, 2014 · 6 comments

Comments

@bulk88
Copy link
Contributor

bulk88 commented Jul 2, 2014

    /* bless into the proper package */
    RETVAL = sv_bless( self, gv_stashpv( class, 0 ) );
    OUTPUT: RETVAL

https://github.com/xsawyerx/xs-fun/blame/master/chapter_03/chapter_03.pod#L170

you should use gv_stashsv instead of gv_stashpv since your input is already a SV, rurban mentioned on #p5p that strlen on the package name could produce an obj blessed into a different package than the one passed in if the package name contains nulls

@xsawyerx
Copy link
Owner

Good point! Will change that soon.

@xsawyerx
Copy link
Owner

When I change it to gv_stashsv, all the tests fail and it can't load it.

Also, I don't understand the comment on strlen.

@bulk88
Copy link
Contributor Author

bulk88 commented Jul 11, 2014

To be precise, to use gv_stashsv

SV *
new( const char *class )

needs to be changed to

SV *
new( SV  *class )

also the 2 mentions of gv_stashpv in the docs need to be then changed to gv_stashsv. You will loose some educational content since after converting to gv_stashsv, there will be zero uses of a "char *" as an incoming param to a xsub in xs-fun (but there still will be a couple uses of char * as an scalar return value in chap 0-2, but those dont really explain how incoming args work).

gv_stashpv("foo\x00\x00",0) and gv_stashpvn("foo\x00\x00", sizeof("foo\x00\x00"), 0) will produce objs from 2 different classes, strlen("foo\x00\x00") is 3, not 5. As an example why null would wind up in a package name, some crazy people quote file paths in Perl with $path = q[NUL]/bin/foo[NUL]; in source code, since null char is the only character that can not be in a file path in POSIX. Until 5.9.3 http://perl5.git.perl.org/perl.git/commitdiff/7423f6db106ad471398838e82e73b22d8c1e166e , package names could not have null in them since they were null terminated.

@xsawyerx
Copy link
Owner

That is pretty cool. Thank you so much for the explanation of strings and nulls in packages.

I can't find a way to incorporate that into the tutorial (feel free to offer, if you have any idea), but I'll change the stashpv to stashsv because it's just correct.

@bulk88
Copy link
Contributor Author

bulk88 commented Jul 14, 2014

In, https://github.com/xsawyerx/xs-fun/blob/master/chapter_03/chapter_03.pod it says

    # Perl on GNU/Linux, BSD, Solaris:
    perl Makefile.PL && make manifest && make distclean

    # Strawberry Perl on Windows:
    perl Makefile.PL
    dmake manifest
    dmake distclean

Windows has "&" and "&&", IDK if their meaning is exactly identical to unix, see http://ss64.com/nt/syntax-conditional.html . If you want for style reasons to make the windows version look more the unix version by putting it all on 1 line, you can.

In https://github.com/xsawyerx/xs-fun/blob/master/chapter_02/chapter_02.pod , IDK if you are writing for a Perl crowd (pointer is a shaky red dot on the projector screen), or a C but no XS/perlapi crowd, but I have to ask the question, what frees the char * returned by chromaprint_get_version()? does it need to be freed? you might want to touch on that topic (even though it is basic C knowledge), don't write to SvPV returned string ptrs. And perl will never free your string * automatically (except for sv_usepvn_flags, and the save stack, but they really dont need to know about those 2 unless you plan to rename this repo to XS-Doctorate)

@paultcochrane
Copy link
Collaborator

Just to make a note of it: the Windows "&&" issue mentioned above was fixed in 517b051.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants