http://qs1969.pair.com?node_id=1180088

stevieb has asked for the wisdom of the Perl Monks concerning the following question:

Given that weekends are typically slower than through the week, I thought I'd ask for a quick code review of those who understand the perl internals. This is my real first attempt at doing anything regarding manipulating and managing perl data types in C/XS, and am just wondering if what I've got below is reasonable, sane and safe, or if there are better ways to do it.

Instead of passing an integer into the C function then doing a bunch of bit shifting to get out the required number of bytes, I wanted to pass in an array reference so that the number of bytes can be dynamic (otherwise with an int, I'm limited to a maximum of four bytes in a call to testing()).

use warnings; use strict; use Inline 'C'; my $channel = 0; my @bytes = (0x00, 0x01, 0x02, 0x03); testing($channel, \@bytes, 4); __END__ __C__ int testing(int channel, SV* byte_ref, int len){ if (channel != 0 && channel != 1){ croak("channel param must be 0 or 1\n"); } if (! SvROK(byte_ref) || SvTYPE(SvRV(byte_ref)) != SVt_PVAV){ croak("not an aref\n"); } AV* bytes = (AV*)SvRV(byte_ref); int num_bytes = av_len(bytes) + 1; if (len != num_bytes){ croak("$len param != elem count\n"); } unsigned char buf[num_bytes]; int i; for (i=0; i<len; i++){ SV** elem = av_fetch(bytes, i, 0); buf[i] = (int)SvNV(*elem); } /* * here, I'll be passing the char buffer and len * to an external C function. For display, I'll * just print the elements (the return is from ioctl()) * * if ((spiDataRW(channel, buf, len) < 0){ * croak("failed to write to the SPI bus\n"); * } */ int x; for (x=0; x<len; x++){ printf("%d\n", buf[x]); } }

Replies are listed 'Best First'.
Re: Is this a sane/safe way to pass an aref into a C function?
by BrowserUk (Patriarch) on Jan 21, 2017 at 21:14 UTC

    I don't see anything particularly wrong with your approach, but I have a few thoughts:

    1. buf[i] = (int)SvNV(*elem);

      buf[] is define as unsigned char, but you're retrieving reals, and casting them to signed ints?

    2. If you are going to pass in an array of ints or reals, you should really check they will fit into an unsigned char or cast appropriately to ensure they will.

      Maybe that should be buf[i] = (unsigned char)SvNV(*elem); or even buf[i] = (unsigned char)SvUV(*elem);

    3. I'll be passing the char buffer and len to an external C function.

      Perl's string scalars can hold any unsigned char value, and carry their length, so why not just pass the pointer and length from a scalar argument?

      That way you cannot be passed out of range values so don't have to deal with them.


    With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority". The enemy of (IT) success is complexity.
    In the absence of evidence, opinion is indistinguishable from prejudice.

      Hey BrowserUK, thanks for responding.

      1) I was casting to int because it was what I had the first time it worked, and hadn't yet done further testing (disclaimer: I am literally learning C as I go through this process).

      2) Yep, I have full intention to ensure that each 'byte' will be < 256 && > -1 or so to speak (ie. a proper uint8_t). This is a piece of a much larger scenario, where I'll be tearing apart the bits and manipulating and in many cases ensuring each bit range is within proper bounds (ie.,eg: are bits 5-4 set correctly) and my lack of understanding of a few things likely had me leave out some items. There will be added checks for everything, but I tried to keep the sample small.

      3) See this node for the why. Again, I'm just coming into the C/XS/bitwise world, so I may not be explaining myself correctly. Essentially, that node shares that I'm working on something where I need a dynamic number of bytes incoming, without having to change the underlying C code, specifically because in some cases, I'll just want to pass the data straight through, other times, I'll need/want to perform bitwise operations on it.

      Really, I summarized this thread into a single problem, where maybe I should have explained it for what it was... looking for a solution to two separate issues while taking up as little time of others as possible ;)

        I need a dynamic number of bytes incoming, without having to change the underlying C code,

        I'm not quite sure why you think you cannot pass variable length strings in scalars?

        Also, whilst pack is one way to encode unsigned bytes into strings, it is not the only way:

        #! perl -slw use strict; use Inline C => Config => BUILD_NOISY => 1; use Inline C => <<'END_C', NAME => '_1180095', CLEAN_AFTER_BUILD =>0; int doStuff( SV *sv ) { STRLEN len, i; unsigned char *bytes = SvPVx( sv, len ); printf( "%d : ", len ); // print the hi & lo nybbles, of the unsigned bytes passed, in hex for( i=0; i < len; ++i ) { printf( "%01x %01x\t", ( bytes[ i ] & 0xf0 ) >> 4, bytes[ i ] +& 0x0f ); } printf( "\n" ); return 1; } END_C doStuff( "\x0f\xf0\xaa\x55\x01\x80" ); doStuff( join '', map chr, 0x00 .. 0xff ); doStuff( pack 'C*', 1 .. 10 );

        Output:

        C:\test>1180095.pl 6 : 0 f f 0 a a 5 5 0 1 8 0 256 : 0 0 0 1 0 2 0 3 0 4 0 5 0 6 0 7 + 0 8 0 9 0 a 0 b 0 c 0 d 0 e 0 f 1 0 + 1 1 1 2 1 3 1 4 1 5 1 6 1 7 1 8 +1 9 1 a 1 b 1 c 1 d 1 e 1 f 2 0 2 1 + 2 2 2 3 2 4 2 5 2 6 2 7 2 8 2 9 2 +a 2 b 2 c 2 d 2 e 2 f 3 0 3 1 3 2 + 3 3 3 4 3 5 3 6 3 7 3 8 3 9 3 a 3 b + 3 c 3 d 3 e 3 f 4 0 4 1 4 2 4 3 4 + 4 4 5 4 6 4 7 4 8 4 9 4 a 4 b 4 c + 4 d 4 e 4 f 5 0 5 1 5 2 5 3 5 4 5 5 + 5 6 5 7 5 8 5 9 5 a 5 b 5 c 5 d +5 e 5 f 6 0 6 1 6 2 6 3 6 4 6 5 6 6 + 6 7 6 8 6 9 6 a 6 b 6 c 6 d 6 e 6 +f 7 0 7 1 7 2 7 3 7 4 7 5 7 6 7 7 + 7 8 7 9 7 a 7 b 7 c 7 d 7 e 7 f 8 0 + 8 1 8 2 8 3 8 4 8 5 8 6 8 7 8 8 +8 9 8 a 8 b 8 c 8 d 8 e 8 f 9 0 9 1 + 9 2 9 3 9 4 9 5 9 6 9 7 9 8 9 9 9 +a 9 b 9 c 9 d 9 e 9 f a 0 a 1 a 2 + a 3 a 4 a 5 a 6 a 7 a 8 a 9 a a a b + a c a d a e a f b 0 b 1 b 2 b 3 b + 4 b 5 b 6 b 7 b 8 b 9 b a b b b c + b d b e b f c 0 c 1 c 2 c 3 c 4 c 5 + c 6 c 7 c 8 c 9 c a c b c c c d +c e c f d 0 d 1 d 2 d 3 d 4 d 5 d 6 + d 7 d 8 d 9 d a d b d c d d d e d +f e 0 e 1 e 2 e 3 e 4 e 5 e 6 e 7 + e 8 e 9 e a e b e c e d e e e f f 0 + f 1 f 2 f 3 f 4 f 5 f 6 f 7 f 8 f + 9 f a f b f c f d f e f f 10 : 0 1 0 2 0 3 0 4 0 5 0 6 0 7 0 8 + 0 9 0 a C:\test>

        With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority". The enemy of (IT) success is complexity.
        In the absence of evidence, opinion is indistinguishable from prejudice.
Re: Is this a sane/safe way to pass an aref into a C function?
by syphilis (Archbishop) on Jan 21, 2017 at 22:56 UTC
    Hi,

    Looks sane to me, but there's a portability issue in that your code requires a C99 compliant compiler.

    It therefore won't build for me with any of my Microsoft compilers - though I don't have a recent MS compiler to test with.
    (The most recent MS compiler I have is  Microsoft (R) C/C++ Optimizing Compiler Version 14.00.40310.41 for AMD64.)

    Even if I pre-declare everything and rewrite your XSub as:
    int testing(int channel, SV* byte_ref, int len){ SV ** elem; AV * bytes = (AV*)SvRV(byte_ref); int num_bytes = av_len(bytes) + 1; unsigned char buf[num_bytes]; int i; int x; if (channel != 0 && channel != 1){ croak("channel param must be 0 or 1\n"); } if (! SvROK(byte_ref) || SvTYPE(SvRV(byte_ref)) != SVt_PVAV){ croak("not an aref\n"); } num_bytes = av_len(bytes) + 1; if (len != num_bytes){ croak("$len param != elem count\n"); } for (i=0; i<len; i++){ elem = av_fetch(bytes, i, 0); buf[i] = (int)SvNV(*elem); } /* * here, I'll be passing the char buffer and len * to an external C function. For display, I'll * just print the elements * * if ((spiDataRW(channel, buf, len) < 0){ * croak("failed to write to the SPI bus\n"); * } */ for (x=0; x<len; x++){ printf("%d\n", buf[x]); } }
    it still errors out with:
    try_pl_1bcb6.xs(10) : error C2057: expected constant expression try_pl_1bcb6.xs(10) : error C2466: cannot allocate an array of constan +t size 0 try_pl_1bcb6.xs(10) : error C2133: 'buf' : unknown size NMAKE : fatal error U1077: 'cl' : return code '0x2' Stop.
    I would work around that by dynamically allocating buf (Newxz/Safefree) but perhaps there's another way.

    Anyway - it's only an issue when (if) your code meets a compiler that's not C99 compliant.

    Cheers,
    Rob

      Thanks syphilis, I never thought about cross-compilation!

      This definitely isn't my end result here, but as I said in an earlier reply, I'm kind of just learning C, so that didn't cross my mind.

      As I still to this day write my Perl code to compile on as far back of a version as I possibly can (5.8.x minimum), this is a nice piece of feedback, and another avenue of research I have to do as I pick up on C. Learning C is important to me as I have a lot of hardware-interaction software I want to write.

      Perlmonks++ for being able to post something only somewhat relative to Perl and getting help for other languages. One thing is for certain... I had an easier time picking up on C pointers than I did originally figuring out Perl references, but while I was learning how to master those (perl refs), I had no programming experience whatsoever. However, this pointer-to-pointer thing (**) is a bit elusive still, so that's another "book learnin'" thing I still have to pick up on ;)

      update: Above, I didn't mean learning Perl references was overly difficult (in fact, they're not... they're quite logical). The comparison is unfair; I hadn't done but a lick of coding when I had to figure out refs in Perl, but once I grasped them, references/pointers/whatever-you-want-to-call-them became immediately understandable. So really, learning Perl refs allowed me to understand the concept behind *any* languages that has the concept of the sort.

Re: Is this a sane/safe way to pass an aref into a C function? (XS<Perl)
by tye (Sage) on Jan 22, 2017 at 17:51 UTC

    XS code is much more likely to be buggy and fragile than Perl code. If you want to pass a list of bytes from Perl to C, then you would be better off doing it as a data structure that doesn't require your C code to try to deal with Perl data structures. This will also have the side-effect of you having less XS code, which will reduce how many bugs you have to fix and how long it will take you to fix the bugs you have.

    So just use:

    ... my $str = pack "U0C*", @bytes; testing( $channel, $str, length($str) ); ... int testing( in­t channel, char* buf, int len ) {

    - tye        

      Thanks tye, that's great!

      I was trying similar things, but did not realize pack was required. This is far easier and less error-prone as you said (as well it saves me from having to wrap one C function with another as this way, I can pass the data directly to the original function), but I am thankful that I was able to learn how to pass in perl variables into a C function, even though I'm going to revert to using this method instead:

      use warnings; use strict; use feature 'say'; use Inline 'C'; my @b = (1..3, 253..255); my $x = pack "V0C*", @b; check($x, length($x)); __END__ __C__ #include <stdio.h> #include <stdint.h> void check(unsigned char* n, int len){ int i; for(i=0; i<len; i++){ printf("%d\n", n[i]); } }

      Output:

      1 2 3 253 254 255

      Of course, in the real code, I perform error checking.

        Good thread.
        I tried to get this code working with Active State 5.20, but I don't have right "formula" to do that on my current little laptop. Is there a FAQ on this? What compiler is necessary, etc...?

        PS: I think your check() would be more "C like" coded something like this:

        void check(unsigned char* n, int len) { while (len--) { printf ("%d\n", *n++); } }
        The variable: int i is unnecessary. With the C calling convention on 32 bit machine, check() is passed a 32 bit pointer to n (pointer to an array of unsigned bytes) and a 32 bit signed integer, len on the stack. These are copies for the subroutine to modify and use as it wishes.

        This n[i] syntax is often coded by the compiler as (n i*size_of_n) plus base address. Whereas n++ is coded as n plus the size_of_n.

Re: Is this a sane/safe way to pass an aref into a C function?
by Anonymous Monk on Jan 21, 2017 at 19:55 UTC

    Hm. A sequence of bytes could be an array in perl but in C it is generally known as a string. :^)

    So you could just pass a string and read it with SvPV. Although I suppose it depends on your concept of the channel. If the bytes are going to have an individual meaning, like a value sampling, then perhaps the array model is appropriate.

      Thanks Anonymonk, the bytes most definitely have individual meaning... MSByte is a control register (which actually consists of two 4-bit nibbles, but I dissect that later in the chain), and LSByte is the data register, both of a larger 16-bit command register, which gets sent to a piece of hardware over the Serial Peripheral Interface.

      Example of what I was originally toying with:

      # perl testing(65535, 4); ... // C #include <stdio.h> int testing(int n, int len){ unsigned char bytes[4]; bytes[3] = (n >> 24) & 0xFF; bytes[2] = (n >> 16) & 0xFF; bytes[1] = (n >> 8) & 0xFF; bytes[0] = n & 0xFF; printf("output should be 255, 255, 0, 0\n\n"); printf("%d, %d, %d, %d\n", bytes[0], bytes[1], bytes[2], bytes[3]) +; return 0; }

      Works fine. The problem though is that the buf array is eventually sent to another C function that puts it on the SPI bus. The way I've done it in the above example limits me to accept/send only a single 4-byte array, so if a device requires say 6 bytes, I'd have to add in another param (at least that's how I saw it). I wanted my Perl code to be as compatible with the API I'm using as possible, and heck, I've always wanted to learn how to pass in an array/aref to a C function anyway, so it was a fun learning experience :)