Skip to content

ext/com_dotnet: Use modern HashTable APIs #16208

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ext/com_dotnet: Use modern HashTable APIs #16208

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86e7e5d
ext/com_dotnet: Use modern HashTable APIs
Girgias Oct 3, 2024
668fd87
Address review comments
Girgias Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 23 additions & 42 deletions ext/com_dotnet/com_variant.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,73 +24,54 @@
#include "php_com_dotnet.h"
#include "php_com_dotnet_internal.h"

/* create an automation SafeArray from a PHP array.
/* create an automation SafeArray from a PHP array (HashTable).
* Only creates a single-dimensional array of variants.
* The keys of the PHP hash MUST be numeric. */
static void safe_array_from_zval(VARIANT *v, zval *z, int codepage)
static void safe_array_from_hashtable(VARIANT *v, HashTable *ht, int codepage)
{
SAFEARRAY *sa = NULL;
SAFEARRAYBOUND bound;
HashPosition pos;
int keytype;
zend_string *strindex;
zend_ulong intindex = 0;
VARIANT *va;
zval *item;

/* find the largest array index, and assert that all keys are integers */
zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(z), &pos);
for (;; zend_hash_move_forward_ex(Z_ARRVAL_P(z), &pos)) {

keytype = zend_hash_get_current_key_ex(Z_ARRVAL_P(z), &strindex, &intindex, &pos);
if (!zend_array_is_list(ht)) {
Copy link
Member

Choose a reason for hiding this comment

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

This is now stricter as before, where arrays with holes could be passed. Since the old behavior doesn't make much sense (would then skip elements at the end), the change might be okay, but the test case would need to be updated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll copy the similar function from f90323c

Copy link
Member

Choose a reason for hiding this comment

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

That would solve the BC problem, but I'm not sure that's the best way forward.

php-src/ext/com_dotnet/tests/variant_variation.phpt

Line 9 in 6deaaf6

"off" => [2 => 1, 1 => 2, 3],

php-src/ext/com_dotnet/tests/variant_variation.phpt

Lines 26 to 28 in 6deaaf6

NULL
int(2)
int(1)

I mean, that doesn't make sense. Might be better to require users to pass a proper list (can be checked upfront, or just pass array_values($my_arr)). At least for PHP 9, this should be done.

// TODO: Make this a ValueError
php_error_docref(NULL, E_WARNING, "COM: converting from PHP array to VARIANT array; only arrays with numeric keys are allowed");
V_VT(v) = VT_NULL;
return;
}
// TODO: Check not empty?

if (HASH_KEY_IS_STRING == keytype) {
goto bogus;
} else if (HASH_KEY_NON_EXISTENT == keytype) {
break;
} else if (intindex > UINT_MAX) {
php_error_docref(NULL, E_WARNING, "COM: max number %u of elements in safe array exceeded", UINT_MAX);
break;
}
uint32_t nb_values = zend_hash_num_elements(ht);
if (nb_values > UINT_MAX) {
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be superfluous, since UINT_MAX == UINT32_MAX for all supported platforms. (Actually, that should have been ULONG_MAX, but that doesn't make a difference on Windows.)

// TODO: Make this a ValueError?
php_error_docref(NULL, E_WARNING, "COM: max number %u of elements in safe array exceeded", UINT_MAX);
V_VT(v) = VT_NULL;
return;
}

/* allocate the structure */
bound.lLbound = 0;
bound.cElements = zend_hash_num_elements(Z_ARRVAL_P(z));
bound.cElements = nb_values;
sa = SafeArrayCreate(VT_VARIANT, 1, &bound);

/* get a lock on the array itself */
SafeArrayAccessData(sa, (void **) &va);
va = (VARIANT*)sa->pvData;

/* now fill it in */
zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(z), &pos);
for (;; zend_hash_move_forward_ex(Z_ARRVAL_P(z), &pos)) {
if (NULL == (item = zend_hash_get_current_data_ex(Z_ARRVAL_P(z), &pos))) {
break;
zend_ulong index = 0;
zval *value;
ZEND_HASH_FOREACH_NUM_KEY_VAL(ht, index, value) {
if (index < bound.cElements) {
php_com_variant_from_zval(&va[index], value, codepage);
}
zend_hash_get_current_key_ex(Z_ARRVAL_P(z), &strindex, &intindex, &pos);
if (intindex < bound.cElements) {
php_com_variant_from_zval(&va[intindex], item, codepage);
}
}
} ZEND_HASH_FOREACH_END();

/* Unlock it and stuff it into our variant */
SafeArrayUnaccessData(sa);
V_VT(v) = VT_ARRAY|VT_VARIANT;
V_ARRAY(v) = sa;

return;

bogus:
php_error_docref(NULL, E_WARNING, "COM: converting from PHP array to VARIANT array; only arrays with numeric keys are allowed");

V_VT(v) = VT_NULL;

if (sa) {
SafeArrayUnlock(sa);
SafeArrayDestroy(sa);
}
}

static void php_com_variant_from_zval_ex(VARIANT *v, zval *z, int codepage, VARTYPE vt)
Expand Down Expand Up @@ -142,7 +123,7 @@ static void php_com_variant_from_zval_ex(VARIANT *v, zval *z, int codepage, VART

case IS_ARRAY:
/* map as safe array */
safe_array_from_zval(v, z, codepage);
safe_array_from_hashtable(v, Z_ARRVAL_P(z), codepage);
break;

case IS_LONG:
Expand Down
105 changes: 32 additions & 73 deletions ext/com_dotnet/com_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,8 @@ static struct IDispatchExVtbl php_dispatch_vtbl = {
* dispatch ids */
static void generate_dispids(php_dispatchex *disp)
{
HashPosition pos;
zend_string *name = NULL;
zval *tmp, tmp2;
int keytype;
zval tmp;
zend_long pid;

if (disp->dispid_to_name == NULL) {
Expand All @@ -435,71 +433,42 @@ static void generate_dispids(php_dispatchex *disp)

/* properties */
if (Z_OBJPROP(disp->object)) {
zend_hash_internal_pointer_reset_ex(Z_OBJPROP(disp->object), &pos);
while (HASH_KEY_NON_EXISTENT != (keytype =
zend_hash_get_current_key_ex(Z_OBJPROP(disp->object), &name,
&pid, &pos))) {
char namebuf[32];
if (keytype == HASH_KEY_IS_LONG) {
snprintf(namebuf, sizeof(namebuf), ZEND_ULONG_FMT, pid);
name = zend_string_init(namebuf, strlen(namebuf), 0);
} else {
zend_string_addref(name);
}

zend_hash_move_forward_ex(Z_OBJPROP(disp->object), &pos);
ZEND_HASH_FOREACH_STR_KEY(Z_OBJPROP(disp->object), name) {
ZEND_ASSERT(name);

/* Find the existing id */
if ((tmp = zend_hash_find(disp->name_to_dispid, name)) != NULL) {
zend_string_release_ex(name, 0);
/* Check ID exists */
if (!zend_hash_exists(disp->name_to_dispid, name)) {
continue;
}

/* add the mappings */
ZVAL_STR_COPY(&tmp2, name);
zend_hash_next_index_insert(disp->dispid_to_name, &tmp2);
pid = zend_hash_next_free_element(disp->dispid_to_name) - 1;

ZVAL_LONG(&tmp2, pid);
zend_hash_update(disp->name_to_dispid, name, &tmp2);
ZVAL_STR_COPY(&tmp, name);
pid = zend_hash_next_free_element(disp->dispid_to_name);
zend_hash_index_update(disp->dispid_to_name, pid, &tmp);

zend_string_release_ex(name, 0);
}
ZVAL_LONG(&tmp, pid);
zend_hash_update(disp->name_to_dispid, name, &tmp);
} ZEND_HASH_FOREACH_END();
}

/* functions */
if (Z_OBJCE(disp->object)) {
zend_hash_internal_pointer_reset_ex(&Z_OBJCE(disp->object)->function_table, &pos);
while (HASH_KEY_NON_EXISTENT != (keytype =
zend_hash_get_current_key_ex(&Z_OBJCE(disp->object)->function_table,
&name, &pid, &pos))) {

char namebuf[32];
if (keytype == HASH_KEY_IS_LONG) {
snprintf(namebuf, sizeof(namebuf), ZEND_ULONG_FMT, pid);
name = zend_string_init(namebuf, strlen(namebuf), 0);
} else {
zend_string_addref(name);
}

zend_hash_move_forward_ex(&Z_OBJCE(disp->object)->function_table, &pos);
ZEND_HASH_FOREACH_STR_KEY(&Z_OBJCE(disp->object)->function_table, name) {
ZEND_ASSERT(name);

/* Find the existing id */
if ((tmp = zend_hash_find(disp->name_to_dispid, name)) != NULL) {
zend_string_release_ex(name, 0);
/* Check ID exists */
if (!zend_hash_exists(disp->name_to_dispid, name)) {
continue;
}

/* add the mappings */
ZVAL_STR_COPY(&tmp2, name);
zend_hash_next_index_insert(disp->dispid_to_name, &tmp2);
pid = zend_hash_next_free_element(disp->dispid_to_name) - 1;

ZVAL_LONG(&tmp2, pid);
zend_hash_update(disp->name_to_dispid, name, &tmp2);
ZVAL_STR_COPY(&tmp, name);
pid = zend_hash_next_free_element(disp->dispid_to_name);
zend_hash_index_update(disp->dispid_to_name, pid, &tmp);

zend_string_release_ex(name, 0);
}
ZVAL_LONG(&tmp, pid);
zend_hash_update(disp->name_to_dispid, name, &tmp);
} ZEND_HASH_FOREACH_END();
}
}

Expand Down Expand Up @@ -540,15 +509,9 @@ static void disp_destructor(php_dispatchex *disp)
CoTaskMemFree(disp);
}

PHP_COM_DOTNET_API IDispatch *php_com_wrapper_export_as_sink(zval *val, GUID *sinkid,
HashTable *id_to_name)
PHP_COM_DOTNET_API IDispatch *php_com_wrapper_export_as_sink(zval *val, GUID *sinkid, HashTable *id_to_name)
{
php_dispatchex *disp = disp_constructor(val);
HashPosition pos;
zend_string *name = NULL;
zval tmp, *ntmp;
int keytype;
zend_ulong pid;

disp->dispid_to_name = id_to_name;

Expand All @@ -558,20 +521,16 @@ PHP_COM_DOTNET_API IDispatch *php_com_wrapper_export_as_sink(zval *val, GUID *si
ALLOC_HASHTABLE(disp->name_to_dispid);
zend_hash_init(disp->name_to_dispid, 0, NULL, ZVAL_PTR_DTOR, 0);

zend_hash_internal_pointer_reset_ex(id_to_name, &pos);
while (HASH_KEY_NON_EXISTENT != (keytype =
zend_hash_get_current_key_ex(id_to_name, &name, &pid, &pos))) {

if (keytype == HASH_KEY_IS_LONG) {

ntmp = zend_hash_get_current_data_ex(id_to_name, &pos);

ZVAL_LONG(&tmp, pid);
zend_hash_update(disp->name_to_dispid, Z_STR_P(ntmp), &tmp);
}

zend_hash_move_forward_ex(id_to_name, &pos);
}
ZEND_ASSERT(zend_array_is_list(id_to_name));
zend_ulong pid;
zval *value;
ZEND_HASH_FOREACH_NUM_KEY_VAL(id_to_name, pid, value) {
ZEND_ASSERT(Z_TYPE_P(value) == IS_STRING);

zval tmp;
ZVAL_LONG(&tmp, pid);
zend_hash_update(disp->name_to_dispid, Z_STR_P(value), &tmp);
} ZEND_HASH_FOREACH_END();

return (IDispatch*)disp;
}
Expand Down
Loading