char CData can't be directly assigned a number

[bwoebi]

Description

The following code:

<?php

$v = FFI::new("char[2]");
$v[0] = 65;
var_dump($v);

Resulted in this output:

Fatal error: Uncaught FFI\Exception: Incompatible types when assigning to type 'char' from PHP 'int' in Command line code:1
Stack trace:
#0 {main}
  thrown in Command line code on line 1

But I expected this output instead:

object(FFI\CData:char[2])#1 (2) {
  [0]=>
  string(1) "A"
  [1]=>
  string(1) ""
}

It's somewhat weird to have to type cast around, i.e. I could just do a FFI::cast to uint8_t * and it works. But for some reason, char is special. Seems like an arbitrary restriction to me, and it really should accept both.

PHP Version

master

Operating System

No response

[cmb69]

It's somewhat weird to have to type cast around, […]

Well, you don't have to; you just need to assign a one-character string (e.g. chr(65)). It might make sense to lift this restriction, but I don't consider the current behavior a serious issue.

[bwoebi]

I agree, it's more of a feature request.

What prompted me to asking for this is that I passed some CData array of integer-y values around, which happen to be defined at different places, with different types and got burned when seeing that.

[SerafimArts]

I propose to do the same not only for chars, but also for strings.

This will solve the problem, for example, with functions like:

void foo(const uint8_t* name);

// real-world example: glXGetProcAddress from OpenGL

Or functions that contain wide strings, like:

// typedef int16_t wchar_t;
// or
// typedef uint16_t wchar_t;

void foo(const wchar_t* name);

// real-world example: Any Win32 API function suffixed by "W"

[nielsdos]

Actually, the current behaviour is very surprising.
Consider the following code:

<?php

$v = FFI::new("char[2]");
$v[0] = 1;
var_dump($v);

This gives:

object(FFI\CData:char[2])#1 (2) {
  [0]=>
  string(1) "1"
  [1]=>
  string(1) ""
}

So you don't get character \1, but you get ASCII 1.
But if your integer consists of more than one digit, then you get an exception.
Changing this would be a BC break, so idk if we want this. In any case I made a patch that implements the new behaviour proposed in the opening post:

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 1398278662..46c87d0dca 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -791,14 +791,19 @@ static zend_always_inline zend_result zend_ffi_zval_to_cdata(void *ptr, zend_ffi
 			*(uint8_t*)ptr = zend_is_true(value);
 			break;
 		case ZEND_FFI_TYPE_CHAR:
-			str = zval_get_tmp_string(value, &tmp_str);
-			if (ZSTR_LEN(str) == 1) {
-				*(char*)ptr = ZSTR_VAL(str)[0];
+			if (Z_TYPE_P(value) == IS_LONG) {
+				*(char*)ptr = (char) Z_LVAL_P(value);
 			} else {
-				zend_ffi_assign_incompatible(value, type);
-				return FAILURE;
+				str = zval_get_tmp_string(value, &tmp_str);
+				if (ZSTR_LEN(str) == 1) {
+					*(char*)ptr = ZSTR_VAL(str)[0];
+					zend_tmp_string_release(tmp_str);
+				} else {
+					zend_tmp_string_release(tmp_str);
+					zend_ffi_assign_incompatible(value, type);
+					return FAILURE;
+				}
 			}
-			zend_tmp_string_release(tmp_str);
 			break;
 		case ZEND_FFI_TYPE_ENUM:
 			kind = type->enumeration.kind;

Regarding @SerafimArts their comment:
Since PHP strings are binary-safe, I think that lifting the restriction for (u)int8_t* makes sense. Lifting the restriction for (u)int16_t* is less senseful because a 16-bit number is not necessarily a part of a string.
A PoC patch for the (u)int8_t case is this (probably with only the minor BC break that previously we had an exception while now it works):

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 46c87d0dca..f9a3507196 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -2600,7 +2600,9 @@ static zend_result zend_ffi_pass_arg(zval *arg, zend_ffi_type *type, ffi_type **
 				return SUCCESS;
 			} else if (Z_TYPE_P(arg) == IS_STRING
 			        && ((ZEND_FFI_TYPE(type->pointer.type)->kind == ZEND_FFI_TYPE_CHAR)
-			         || (ZEND_FFI_TYPE(type->pointer.type)->kind == ZEND_FFI_TYPE_VOID))) {
+			         || (ZEND_FFI_TYPE(type->pointer.type)->kind == ZEND_FFI_TYPE_VOID)
+			         || (ZEND_FFI_TYPE(type->pointer.type)->kind == ZEND_FFI_TYPE_UINT8)
+			         || (ZEND_FFI_TYPE(type->pointer.type)->kind == ZEND_FFI_TYPE_SINT8))) {
 				*(void**)arg_values[n] = Z_STRVAL_P(arg);
 				return SUCCESS;
 			} else if (Z_TYPE_P(arg) == IS_OBJECT && Z_OBJCE_P(arg) == zend_ffi_cdata_ce) {

So the question is do we want this (do we want both patches or only the first one?), and does this change need an RFC?