ext/sockets: GH-20532 socket_addrinfo_lookup() return EAI error code on #20534
Conversation
devnexen
force-pushed
the
gh20532
branch
2 times, most recently
from
2bd7801 to
2ae99a8
Compare
…de on resolution failures.
devnexen
force-pushed
the
gh20532
branch
6 times, most recently
from
11a0841 to
d90d227
Compare
| if (getaddrinfo(ZSTR_VAL(hostname), service, &hints, &result) != 0) { | ||
| RETURN_FALSE; | ||
| if ((ret = getaddrinfo(ZSTR_VAL(hostname), service, &hints, &result)) != 0) { | ||
| RETURN_LONG(ret); |
There was a problem hiding this comment.
This is a BC break that might be quite problematic because users often do === false check so I don't think this acceptable in minor. You will need RFC for this.
There was a problem hiding this comment.
so I don't think this acceptable in minor.
not sure what do you mean by minor.
There was a problem hiding this comment.
Ah ok you would had accepted if it was PHP 9.0 rather 8.6 ?
There was a problem hiding this comment.
I guess it would be more acceptable but not sure if it would be good idea either. The problem is that those BC breaks are really hard to catch so I would prefer RFC for it anyway.
|
Alternative solution could be a by ref param but not sure if it's really nice but would be safer and could still expose the contant. Not sure if we got any function that does it like it though? |
|
sure I ll create a RFC. will probably go with the additional optional parameter then. |
|
If it changes to additional parameter, I wouldn't strictly require RFC but might be still good to at least raise at internals to see if there are any objections. |
|
Would it be possible to use |
|
I do not think this it's a good idea, last_error is for errno we would be mixing semantics ; also while EAI* constants are negative on Linux, they have positive values on macos/freebsd for example. |
3 participants
resolution failures.