review object management

[remicollet]

This fix segfault on some arches, see #159

Sctach build with PHP 7.4 on Fedora Rawhide on i686, armv7hl, ppc64le, x86_64, aarch64 and s390x
https://koji.fedoraproject.org/koji/taskinfo?taskID=53905129

[remicollet]

Sctach build with PHP 7.3 on Fedora 31 on armv7hl, x86_64, i686, aarch64, ppc64le and s390x
https://koji.fedoraproject.org/koji/taskinfo?taskID=53906459

P.S. the build includes the run of the test suite.

[remicollet]

Local test on i686 and x86_64 with PHP 7.2 and 8.0, everything also OK.

[rtheunissen]

Thank you for working on this. I would like to know why having std as the first member was causing a problem on some environments when none of these classes support properties and are final. I support this change, just want to make sure I understand what's happening.

[rtheunissen]

Do you think it's possible that some of the peripheral changes fixed the issue, rather than the object management itself?

[remicollet]

Do you think it's possible that some of the peripheral changes fixed the issue, rather than the object management itself?

Yes, probably, but was not able to find which one.
Also segfault occurs differently across version (7.3, 7.4, 8.0)... and arch.

BTW, in all case, pointer cast are evil ;)

[cmb69]

It seems to the missing return statements in php_common_handlers.c are a serious issue.

[rtheunissen]

Yes, probably, but was not able to find which one.

I think we std was first and xtoffset is 0, with the object accessors using casting we would know whether the peripheral changes fixed the issue or not. I do intend to merge this though, I agree that following the convention is wise, I'm just aware of how this then impacts ext-decimal and designs for ext-ds 2.0 heavily relying on struct hacks to reduce indirection.

BTW, in all case, pointer cast are evil ;)

Why is that?

[remicollet]

BTW, in all case, pointer cast are evil ;)

Why is that?

Different types have different memory layout, content, size, ...
So, generally, casting a pointer is bad, with very few exceptions
So in all cases must be considered careffully, and avoid as much as possible.

[rtheunissen]

@remicollet I'm not set up to test on the environments where this was failing before, and I would like to understand what these changes are fixing exactly. Could we maybe increasingly reduce the scope of the changes until a minimal patch is available? I would like to know if it definitely is the object management that was tripping up.

[remicollet]

The minimall patch is the php_common_handlers part (missing return)

[remicollet]

Replaced by pr #165

[rtheunissen]

The minimall patch is the php_common_handlers part (missing return)
Thank you for compiling that.

How is #165 different to this / why close?

[remicollet]

#165 is the same
This one appears corrupt.... (git mystery)