|
|
||
| if (ret_code) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this evaluate to false if ret_code === 0?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, is exec not also affected by this?
| if (ret_code) { | |
| ZEND_TRY_ASSIGN_REF_LONG(ret_code, ret); | |
| } |
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TL;DR: this code is fine.
[First, that's C not PHP, so there's no "===" operator; second,] ret_code is a zval * i.e. a pointer to zval, so if (ret_code) means if (ret_code != NULL) i.e. (here) the PHP function was called with a 2nd argument (a variable into which to copy the value of ret_pclose).
Member
This needs some tests.
…enerate arginfo
Contributor
Sorry to reply here, I'm not subscribed to the mailing list yet, will fix that during the weekend.
I added a 👎 because I think that adding referenced parameters adds confusion everywhere they are used.
The feature is definitely useful, but I think it should be achieved in a different way, in a more "pure" (in terms of function).
There are many options for that:
- Returning a structured array - this would be a definitive major BC break and an ugly solution.
- Returning a
Stringablestructural object - This would be a better solution and a minor BC break - Other ideas?
| ?> | ||
| --EXPECT-- | ||
| int(0) | ||
| int(127) |
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this 127 on unix-like OS's (Linux, MacOS, BSD),
but just 1 on Windows ? then again i wouldn't expect to find /bin/true on Windows
it is possible to make /bin/true executable on windows tho:
C:\>mkdir bin
C:\>cd bin
C:\bin>copy C:\Windows\System32\PING.EXE .
1 file(s) copied.
C:\bin>move PING.EXE true.exe
1 file(s) moved.
C:\bin>php -r "var_dump(is_executable('/bin/true'));"
bool(false)
C:\bin>move true.exe true
1 file(s) moved.
C:\bin>php -r "var_dump(is_executable('/bin/true'));"
bool(true)
Member
What's the status here? The respective RFC is still listed as being under discussion, although apparently there was no more discussion for quite a while.
Are you planning to continue the RFC process for this feature, @l-pt?
Member
Actually, I don't see the point getting the result code with shell_exec(). It is already possible to use exec() instead, and since shell_exec() is equivalent to the backtick operator, how could this parameter be mapped to the operator?
Contributor
@cmb69: A point argued in https://bugs.php.net/bug.php?id=81493 was:
to use
exec()for the same task asshell_exec(), you'll have to (ab)useimplode()likeexec($cmd,$output,$ret); $output=implode("\n", $output);and to use
system()for the same job, you'd have to (ab)useob_*likeob_start(); system($cmd,$ret); $output=ob_get_clean();same for
passthru()
and https://externals.io/message/116439 added:
makes sense for consistency
and I wonder if "Execute an external program" is really always identical to "Execute command via shell" (e.g. for shell builtins)?
As for the backtick operator, it is explicitly said to remain unaffected.
Member
and I wonder if "Execute an external program" is really always identical to "Execute command via shell" (e.g. for shell builtins)?
exec(), system(), passthru() and shell_exec() are wrappers over popen(3); as such, they should be interchangeable (except for the details you've mentioned, and additionally the use of different text/binary modes on Windows).