Skip to content

Implement GH-11934: Allow to pass CData into struct and/or union fields #11935

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

Closed
wants to merge 6 commits into from

Conversation

nielsdos
Copy link
Member

Implementation based on a discussion with KapitanOczywisty.

…ields

Implementation based on a discussion with KapitanOczywisty.
@KapitanOczywisty
Copy link

KapitanOczywisty commented Aug 11, 2023

This function changed quite a few things, so more tests could be added. I don't know what is the windows equivalent for the last one, and likely it is not the best test either.

I've tested all of them in 8.2 (fail) and 8.3 (success).

Tests

<?php

echo "--- Callback return type --- \n";

$ffi = FFI::cdef('
typedef uint32_t (*test_callback)();
typedef struct {
	test_callback call_me;
} my_struct;
');

$struct = $ffi->new('my_struct');
$struct->call_me = function () use ($ffi) {
	$int = $ffi->new('uint32_t');
	$int->cdata = 42;
	return $int;
};

var_dump(($struct->call_me)());

echo "--- Other FFI\CData assignment ---\n";

$ffi = FFI::cdef('');

$source = $ffi->new('uint32_t');
$source->cdata = 123;
$target = $ffi->new('uint32_t');
$target->cdata = $source;

var_dump($target->cdata);

echo "--- Array element ---\n";

$ffi = FFI::cdef('');

$source = $ffi->new('uint32_t');
$source->cdata = 123;
$target = $ffi->new('uint32_t[1]');
$target[0] = $source;

var_dump($target[0]);

echo "--- Existing C variables --- \n";

$ffi = FFI::cdef(
	"int errno;",
	"libc.so.6"
);
$ffi->errno = 0;
var_dump($ffi->errno);
$source = $ffi->new('int');
$source->cdata = 31;
$ffi->errno = $source;
var_dump($ffi->errno);

@bukka
Copy link
Member

bukka commented Aug 11, 2023

Seem like small sefl contained change so if you manage to get approval from @dstogov (maintainer) before the first RC, I would be fine with this going to 8.3.

Co-authored-by: KapitanOczywisty <[email protected]>
@nielsdos
Copy link
Member Author

@KapitanOczywisty Good call, in particular the subtle difference of the callback is clearly a candidate for testing. I've added all your tests in an additional commit, with your co-authored-by tag to credit you.

@KapitanOczywisty
Copy link

@nielsdos I found in other ffi tests how to do C variable test properly: KapitanOczywisty@f3d6246

And callback test is duplicated on purpose?

@nielsdos
Copy link
Member Author

@nielsdos I found in other ffi tests how to do C variable test properly: KapitanOczywisty@f3d6246

Neat, I've added your commit, thanks.

And callback test is duplicated on purpose?

No, sorry for the mess. I was experimenting with creating a variation earlier and didn't fully undo my changes.

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge this into PHP-8.3.

@nielsdos nielsdos closed this in 0b9702c Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FFI] Allow to pass CData into struct and/or union fields.
4 participants