Everything

php

Migrating php extension from 5 to 7, ZVAL_STRINGL no value returned

php.internals

Richard Klingler

8 years ago

EHLO (o;

Seems either my first post was blocked or got stuck as news.php.net
always reports some errors...anyway..


I am trying to migrate a simple php extension where a function is called
in php like:

$error = readmydevice($device, $string, $length);

$string is supplied as empty string and is filled inside the function
with bytes returned from a hardware.

Now this worked fine with php-5.x...


The function looks like:

ZEND_FUNCTION(readmydevice)
{
zend_long n;
zval *z;
size_t len;
char *p;
long r;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "lzl", &n,
&z, &len) == FAILURE) {
return;
}

p = (char *) emalloc(len + 1);
memset(p,0,len+1);

r = myread(n, p, len);
printf("Read %d bytes from device %d up to %d bytes\n", mycount,
n, len);
p[ibcnt] = '\0';
ZVAL_STRINGL(z, p, mycount);
printf("Returned %s\n", p);
efree(p);
RETURN_LONG(r);
}


Any ideas why ZVAL_STRINGL is not doing anything?
Or did I miss something?


thanks in advance
richard

Sara Golemon

8 years ago

On Mon, Mar 12, 2018 at 12:16 PM, Richard Klingler <richard@klingler.net> wrote:

> ZEND_FUNCTION(readmydevice)
> {
>         zend_long n;
>         zval *z;
>         size_t len;
>         char *p;
>         long r;
>
>         if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "lzl", &n,
> &z, &len) == FAILURE) {
>                 return;
>         }
>
>         p = (char *) emalloc(len + 1);
>         memset(p,0,len+1);
>
>         r = myread(n, p, len);
>         printf("Read %d bytes from device %d up to %d bytes\n", mycount,
> n, len);
>         p[ibcnt] = '\0';
>         ZVAL_STRINGL(z, p, mycount);
>         printf("Returned %s\n", p);
>         efree(p);
>         RETURN_LONG(r);
> }
>
>
> Any ideas why ZVAL_STRINGL is not doing anything?
> Or did I miss something?
>

Well, ZVAL_STRINGL is probably doing something, but there are two
reasons there's no way to be certain:

1. What is 'mycount' and where does it come from / how is it set?  If
it's zero, then you'll be saving a zero length string. In any case, it
doesn't seem to have any relationship to the data read into `p`.  I'm
guessing you wanted to use `r` rather than `mycount` both here and in
your debug message.

2. I'm inferring that you want argument 2 to your function to be a
pass-by-ref argument.  Are you specifying any arg_info in the function
entry list? (where you have your PHP_FE macro -- or perhaps ZEND_FE
macro, since you seem to be using those versions).  In order to pass
by ref, you need that structure to specify that the argument is
by-ref.  This *might* have worked without it in PHP 5, but only by
accident and would have probably failed in hard-to-diagnose ways.

2a. Also, when using by-ref args, you need to destruct any previously
held value.  In this case, with a zval_dtor(z); prior to the
ZVAL_STRINGL() macro.

-Sara

Richard Klingler

8 years ago

Hello Sara

Seems my post to news.php.net went somehow through though my NNTP client complained about errors...anyway (o;

"mycount" is defined in here:

FUN_ACCESSOR(mycount)

Which is the updated by the read function.

The function macro itself is defined as:

ZEND_FE(readmydevice,		NULL)

So would the arg_info go in here? There isn't any arg_info in the original code.

thanks in advance
richard

On Mon, 12 Mar 2018 13:18:30 -0400, Sara Golemon wrote:

Sara Golemon

8 years ago

On Tue, Mar 13, 2018 at 3:52 AM, Richard Klingler <richard@klingler.net> wrote:

> Hello Sara
> The function macro itself is defined as:
>
>         ZEND_FE(readmydevice,           NULL)
>
> So would the arg_info go in here? There isn't any arg_info in the original code.
>

Yep.  You'll want something like this:


ZEND_BEGIN_ARG_INFO(arginfo_readmydevice, 0)
ZEND_ARG_INFO(0, device)
ZEND_ARG_INFO(1, string)
ZEND_ARG_INFO(0, length)
ZEND_END_ARG_INFO();
ZEND_FUNCTION(readmydevice) {

/* do your stuff to build p and mycount */

zval_dtor(z);
ZVAL_STRINGL(z, p, mycount);

/* finish up */
}

zend_function_entry whateveryoucallit[] = {
/* other FEs.... */
ZEND_FE(readmydevice, arginfo_readmydevice)
/* other FEs... */
ZEND_FE_END
};



That "1" in ZEND_ARG_INFO(1, string) is what tells the engine that the
argument is pass-by-ref.  You should have had this in PHP 5 as well.
It *appeared* to work purely by the accident of how you were calling
it from userspace.  The reality is that you could easily have wound up
with memory leaks and/or "randomly" changing values unrelated to your
initial variable.

I assume your extension isn't open source or you'd have linked it by
now, but if you can share it, I'd be happy to offer other suggestions.
Based on what I've seen so far, I'm willing to bet there are other
small, subtle bugs hiding in there.

-Sara

P.S. - Side benefit of defining these arginfo structures is that
ReflectionFunction will actually give you parameter info now.