Skip to content

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

Closed
ndossche wants to merge 6 commits into
php:masterfrom
ndossche:ffi-cdata
Closed

ndossche wants to merge 6 commits into
php:masterfrom
ndossche:ffi-cdata

Conversation

@ndossche

Copy link
Copy Markdown
Member

Implementation based on a discussion with KapitanOczywisty.

…ields

Implementation based on a discussion with KapitanOczywisty.
@KapitanOczywisty

KapitanOczywisty commented Aug 11, 2023 •

Copy link
Copy Markdown

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

bukka commented Aug 11, 2023

Copy link
Copy Markdown
Member

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]>
@ndossche

Copy link
Copy Markdown
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
Copy Markdown

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

And callback test is duplicated on purpose?

@ndossche

Copy link
Copy Markdown
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.

Comment thread ext/ffi/tests/gh11934b.phpt Resolved

@dstogov dstogov left a comment

Copy link
Copy Markdown
Member

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.

@ndossche ndossche 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