Hey:
We use SUCCESS/FAILURE as return value in some APIs, but use
0/1(false/true) in others.
I'd like to remove SUCCESS/FAILURE at all, use 0/1 instead..
what do you think?
thanks
--
Xinchen Hui
@Laruence
http://www.laruence.com/
Hi!
Hey:
We use SUCCESS/FAILURE as return value in some APIs, but use
0/1(false/true) in others.I'd like to remove SUCCESS/FAILURE at all, use 0/1 instead..
what do you think?
I think it would make reading code harder. Why do it - is there any
benefit in it? SUCCESS/FAILURE is a good way to indicate - well, success
and failure. Replacing it with meaningless numbers would make code
harder to read and harder to write without mistakes.
Stas Malyshev
smalyshev@gmail.com
Hey
Hi!
Hey:
We use SUCCESS/FAILURE as return value in some APIs, but use
0/1(false/true) in others.I'd like to remove SUCCESS/FAILURE at all, use 0/1 instead..
what do you think?
I think it would make reading code harder. Why do it - is there any
benefit in it? SUCCESS/FAILURE is a good way to indicate - well, success
and failure. Replacing it with meaningless numbers would make code
harder to read and harder to write without mistakes.
I think if(func()) is better, more readeable than if(func() == success)
Thanks
--
Stas Malyshev
smalyshev@gmail.com
Hi!
I think if(func()) is better, more readeable than if(func() == success)
Not really, especially given the fact that for major part of libc
if(func()) means checking for error, not for success, as success value is 0.
--
Stas Malyshev
smalyshev@gmail.com
Hey
Hi!
Hey:
We use SUCCESS/FAILURE as return value in some APIs, but use
0/1(false/true) in others.I'd like to remove SUCCESS/FAILURE at all, use 0/1 instead..
what do you think?
I think it would make reading code harder. Why do it - is there any
benefit in it? SUCCESS/FAILURE is a
yes， to be consistent
We have functions all return int
But some is them use success (0) some of them use bool true(1)
That will lead to hard to write codes
Thanks
good way to indicate - well, success
and failure. Replacing it with meaningless numbers would make code
harder to read and harder to write without mistakes.
I think if(func()) is better, more readeable than if(func() == success)Thanks
--
Stas Malyshev
smalyshev@gmail.com
Hey:
We use SUCCESS/FAILURE as return value in some APIs, but use
0/1(false/true) in others.I'd like to remove SUCCESS/FAILURE at all, use 0/1 instead..
what do you think?
thanks
Hi,
Honestly, I don’t think SUCCESS and FAILURE are bad, they make it explicit that some operation is taking place. But using int as the return type seems odd, maybe we could add some zend_ type for this, maybe an alias of zend_bool?
Similarly, though zend_uchar seems to be what’s used most of the time to store the return value of Z_TYPE(), maybe something like zend_type might be good.
In fact, this is exactly what enums do, maybe we should use one:
typedef enum _zend_success {
FAILURE = 0,
SUCCESS = 1
} zend_success;
Thanks.
Andrea Faulds
http://ajf.me/
Hey:
thanks, that is the first thought of mine too.
Hey:
We use SUCCESS/FAILURE as return value in some APIs, but use
0/1(false/true) in others.I'd like to remove SUCCESS/FAILURE at all, use 0/1 instead..
what do you think?
thanks
Hi,
Honestly, I don’t think SUCCESS and FAILURE are bad, they make it explicit that some operation is taking place. But using int as the return type seems odd, maybe we could add some zend_ type for this, maybe an alias of zend_bool?
Similarly, though zend_uchar seems to be what’s used most of the time to store the return value of Z_TYPE(), maybe something like zend_type might be good.
In fact, this is exactly what enums do, maybe we should use one:
typedef enum _zend_success {
FAILURE = 0,
SUCCESS = 1
} zend_success;
For now, we can not tell what the function return to represent status
from the declaration , since they all return int.
int func();
we have to looks into the source code to get what it used.
so change int func() to zend_success func() may help some bits.
But: return 0 and return FAILURE... which is simpler?
thanks
Thanks.
Andrea Faulds
http://ajf.me/
--
Xinchen Hui
@Laruence
http://www.laruence.com/
Hi!
But: return 0 and return FAILURE... which is simpler?
It's equally simple to write, but FAILURE of course is way simpler to
understand when read.
--
Stas Malyshev
smalyshev@gmail.com
Hi!
But: return 0 and return FAILURE... which is simpler?
It's equally simple to write, but FAILURE of course is way simpler to
understand when read.
I totally agree.
I do not care much about the value of failure or success but I am tired to
have to read the code to see if it is 0, 1, or -1 on failure.
The kind of uniformization I would like to see for the php internals APIs.
About the argument for the lack of info in function signature:
A simple typedef will solve it, for the good:
status php_foo();
Or something along this line.
Yes, it will mean yet another large set of changes for ext developers. But
at this point, it may be a good time to do it.
Cheers,
Pierre
Hi!
But: return 0 and return FAILURE... which is simpler?
It's equally simple to write, but FAILURE of course is way simpler to
understand when read.I totally agree.
I do not care much about the value of failure or success but I am tired to
have to read the code to see if it is 0, 1, or -1 on failure.The kind of uniformization I would like to see for the php internals APIs.
About the argument for the lack of info in function signature:
A simple typedef will solve it, for the good:
status php_foo();
Or something along this line.
Yes, it will mean yet another large set of changes for ext developers. But
at this point, it may be a good time to do it.
hmm, okey
so, make the functions which use SUCCESS/FAILURE return php_success type?
and maybe also typedef php_success php_status?
thanks
thanks
Cheers,
Pierre
--
Xinchen Hui
@Laruence
http://www.laruence.com/
Hi!
But: return 0 and return FAILURE... which is simpler?
It's equally simple to write, but FAILURE of course is way simpler to
understand when read.I totally agree.
I do not care much about the value of failure or success but I am tired to
have to read the code to see if it is 0, 1, or -1 on failure.The kind of uniformization I would like to see for the php internals APIs.
About the argument for the lack of info in function signature:
A simple typedef will solve it, for the good:
status php_foo();
Or something along this line.
Yes, it will mean yet another large set of changes for ext developers. But
at this point, it may be a good time to do it.
hmm, okeyso, make the functions which use SUCCESS/FAILURE return php_success type?
and maybe also typedef php_success php_status?
more like:
#include <stdio.h>
/* From http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_types.h#48 /
typedef enum {
SUCCESS = 0,
FAILURE = -1, / this MUST stay a negative number, or it may
affect functions! */
} ZEND_RESULT_CODE;
typedef enum ZEND_RESULT_CODE status;
status func (status s) {
printf("%s\n", s == FAILURE ? "FAILURE" : "SUCCESS");
return s;
}
int main (void) {
status s;
s = SUCCESS;
func (s);
s = FAILURE;
func(s);
return 0;
}
according to the current way FAILURE/SUCCESS are defined.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Hey:
Hi!
But: return 0 and return FAILURE... which is simpler?
It's equally simple to write, but FAILURE of course is way simpler to
understand when read.
I can not agree with that since nowdays, false === 0, true === 1 is
almost a common sense.
thanks
--
Stas Malyshev
smalyshev@gmail.com
--
Xinchen Hui
@Laruence
http://www.laruence.com/
typedef enum _zend_success {
FAILURE = 0,
SUCCESS = 1
} zend_success;
mysqlnd uses a enum for that already. See
http://lxr.php.net/xref/PHP_TRUNK/ext/mysqlnd/mysqlnd_enum_n_def.h#139
If a PHP-wide thing is introduced this should be unified.
johannes
typedef enum _zend_success {
FAILURE = 0,
SUCCESS = 1
} zend_success;mysqlnd uses a enum for that already. See
http://lxr.php.net/xref/PHP_TRUNK/ext/mysqlnd/mysqlnd_enum_n_def.h#139
Very good example of double definition for nothing :)
If a PHP-wide thing is introduced this should be unified.
Indeed, see my previous reply
johannes
Hmm. This thread doesn't seem to mention it, but why must failure be
negative? I understand the non-zero part but not negative. Aside from
the fact we probably have code relying on it to be negative at this
point is there some other reason?
Hmm. This thread doesn't seem to mention it, but why must failure be
negative? I understand the non-zero part but not negative. Aside from
the fact we probably have code relying on it to be negative at this
point is there some other reason?
Hey Levi,
I believe it's a convention among C APIs: 0 for success, negative for some error
Hmm. This thread doesn't seem to mention it, but why must failure be
negative? I understand the non-zero part but not negative. Aside from
the fact we probably have code relying on it to be negative at this
point is there some other reason?Hey Levi,
I believe it's a convention among C APIs: 0 for success, negative for some error
I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.
Thanks.
Hmm. This thread doesn't seem to mention it, but why must failure be
negative? I understand the non-zero part but not negative. Aside from
the fact we probably have code relying on it to be negative at this
point is there some other reason?Hey Levi,
I believe it's a convention among C APIs: 0 for success, negative for
some errorI'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.
The values do not matter but a consistent usage across all internal APIs.
Hmm. This thread doesn't seem to mention it, but why must failure be
negative? I understand the non-zero part but not negative. Aside from
the fact we probably have code relying on it to be negative at this
point is there some other reason?Hey Levi,
I believe it's a convention among C APIs: 0 for success, negative for some error
I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.
Actually, that email was cut off: unfortunately iOS doesn’t require confirmation to send an email and my finger slipped.
The main point of negative values AIUI, is that you can return useful information for positive or zero values and error codes for negative values. For example with strpos(). For some APIs you only need 0 and -1, mind you, but being able to do if (foo() < 0) for everything is pretty neat.
Andrea Faulds
http://ajf.me/
I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.
My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.
(while there are cases where -1 is valid, see recent famous pid
= fork(); /* ... */ kill(pid, SIGKILL); issue)
johannes
On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:
I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famous pid = fork(); /* ... */ kill(pid, SIGKILL); issue)
I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to do strpos() == SUCCESS to check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?
For the record I am in favor of an enum such as zend_status or some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.
On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famous pid = fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.
Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other functions which use negative return values on error. Some functions return negative error codes, others just -1. Some functions return useful positive values, others just 0. But the idea is that all functions return a negative number on error, so you can use if (foo() < 0) to check for errors. That’s the point of making FAILURE be -1, AIUI. It makes it consistent with other things, like fork() or strpos().
Thanks.
--
Andrea Faulds
http://ajf.me/
Hey:
On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famous pid = fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other functions which use negative return values on error. Some functions return negative error codes, others just -1. Some functions return useful positive values, others just 0. But the idea is that all functions return a negative number on error, so you can use if (foo() < 0) to check for errors. That’s the point of making FAILURE be -1, AIUI. It makes it consistent with other things, like fork() or
strpos().Thanks.
lets make this simple.
first we need unify PHP self..
thanks
--
Andrea Faulds
http://ajf.me/
--
Xinchen Hui
@Laruence
http://www.laruence.com/
Hey:
On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famous pid = fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other functions which use negative return values on error. Some functions return negative error codes, others just -1. Some functions return useful positive values, others just 0. But the idea is that all functions return a negative number on error, so you can use if (foo() < 0) to check for errors. That’s the point of making FAILURE be -1, AIUI. It makes it consistent with other things, like fork() or
strpos().Thanks.
lets make this simple.first we need unify PHP self..
Exactly, and it is not like we can unify libc ourselves ;)
--
Pierre
@pierrejoye | http://www.libgd.org
On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famous pid = fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other functions which use negative return values on error. Some functions return negative error codes, others just -1. Some functions return useful positive values, others just 0. But the idea is that all functions return a negative number on error, so you can use if (foo() < 0) to check for errors. That’s the point of making FAILURE be -1, AIUI. It makes it consistent with other things, like fork() or
strpos().
doing if (foo() < 0 is exactly what should not be done, for any
function returning a status. Only FAILURE and SUCCESS should be used.
Which value FAILURE and SUCCESS have is not really relevant here but
to actually be consistent.
For example
ZEND_API int zend_hash_del(HashTable *ht, zend_string *key)
should actually be
ZEND_API status zend_hash_del(HashTable *ht, zend_string *key)
and its usage should be:
if (zend_hash_del(ht, key) == FAILURE) {
...
}
Same for zend_parse_parameters and the likes.
However functions like zval_update_class_constant
(http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_API.c#1132 ) and all the
underlying functions, are confusing. Both the signature and the return
values should rely on FAILURE/SUCCESS.
I think this is what Xinchen means too. Or at least this is what I
mean with unify the APIs.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Hey:
On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid. Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famous pid = fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other functions which use negative return values on error. Some functions return negative error codes, others just -1. Some functions return useful positive values, others just 0. But the idea is that all functions return a negative number on error, so you can use if (foo() < 0) to check for errors. That’s the point of making FAILURE be -1, AIUI. It makes it consistent with other things, like fork() or
strpos().doing if (foo() < 0 is exactly what should not be done, for any
function returning a status. Only FAILURE and SUCCESS should be used.Which value FAILURE and SUCCESS have is not really relevant here but
to actually be consistent.For example
ZEND_API int zend_hash_del(HashTable *ht, zend_string *key)
should actually be
ZEND_API status zend_hash_del(HashTable *ht, zend_string *key)
and its usage should be:
if (zend_hash_del(ht, key) == FAILURE) {
...
}Same for zend_parse_parameters and the likes.
However functions like zval_update_class_constant
(http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_API.c#1132 ) and all the
underlying functions, are confusing. Both the signature and the return
values should rely on FAILURE/SUCCESS.I think this is what Xinchen means too. Or at least this is what I
mean with unify the APIs.
yes. and as a soft solution.
we can change these functions which use success/failure return
zend_status instead of int first.
thanks
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
--
Xinchen Hui
@Laruence
http://www.laruence.com/
There's already ZEND_RESULT_CODE, or did I miss anything?
Hey:
On Thu, Dec 25, 2014 at 12:38 PM, Pierre Joye pierre.php@gmail.com
wrote:On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid.
Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famouspid
= fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other
functions which use negative return values on error. Some functions return
negative error codes, others just -1. Some functions return useful positive
values, others just 0. But the idea is that all functions return a negative
number on error, so you can use if (foo() < 0) to check for errors. That’s
the point of making FAILURE be -1, AIUI. It makes it consistent with other
things, like fork() orstrpos().doing if (foo() < 0 is exactly what should not be done, for any
function returning a status. Only FAILURE and SUCCESS should be used.Which value FAILURE and SUCCESS have is not really relevant here but
to actually be consistent.For example
ZEND_API int zend_hash_del(HashTable *ht, zend_string *key)
should actually be
ZEND_API status zend_hash_del(HashTable *ht, zend_string *key)
and its usage should be:
if (zend_hash_del(ht, key) == FAILURE) {
...
}Same for zend_parse_parameters and the likes.
However functions like zval_update_class_constant
(http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_API.c#1132 ) and all the
underlying functions, are confusing. Both the signature and the return
values should rely on FAILURE/SUCCESS.I think this is what Xinchen means too. Or at least this is what I
mean with unify the APIs.
yes. and as a soft solution.we can change these functions which use success/failure return
zend_status instead of int first.thanks
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
--
Xinchen Hui
@Laruence
http://www.laruence.com/
There's already ZEND_RESULT_CODE, or did I miss anything?
Yes, to read the thread :)
We are not talking about the lack of a status typedef but about the
inconsistencies across PHP internal APIs. And what Xinchen suggests
below is to begin by changing the functions signature from int to
status (aka ZEND_RESULT_CODE, see my previous code example) as a 1st
step.Functions returning 0/1/-1 themselves can be changed as a 2nd
step.
we can change these functions which use success/failure return
zend_status instead of int first.
There's already ZEND_RESULT_CODE, or did I miss anything?
According to lxr.php.net, this symbol ZEND_RESULT_CODE is not
referenced in any place except its definition. We can begin using it
if we like, or even rename it. Theoretically renaming it may break
extensions; none that I could find reference it, though.
There's already ZEND_RESULT_CODE, or did I miss anything?
According to lxr.php.net, this symbol ZEND_RESULT_CODE is not
referenced in any place except its definition. We can begin using it
if we like, or even rename it. Theoretically renaming it may break
extensions; none that I could find reference it, though.
Hey Levi,
I think we should just rename it, it's a little too long. zend_status or ZEND_STATUS would be fine with me, preferably the former since we tend to use lowercase type names. Another possibility might be zend_result.
If you folks like, I could go ahead and write a patch for master to rename it and use it in a bunch of places.
Thanks.
--
Andrea Faulds
http://ajf.me/
Hey:
There's already ZEND_RESULT_CODE, or did I miss anything?
According to lxr.php.net, this symbol ZEND_RESULT_CODE is not
referenced in any place except its definition. We can begin using it
if we like, or even rename it. Theoretically renaming it may break
extensions; none that I could find reference it, though.Hey Levi,
I think we should just rename it, it's a little too long. zend_status or ZEND_STATUS would be fine with me, preferably the former since we tend to use lowercase type names. Another possibility might be zend_result.
zend_status +1
If you folks like, I could go ahead and write a patch for master to rename it and use it in a bunch of places.
great, thanks... please only added it to these functions who already
use success/failure, don't change others :)
thanks
Thanks.
--
Andrea Faulds
http://ajf.me/
--
Xinchen Hui
@Laruence
http://www.laruence.com/
Hey:
There's already ZEND_RESULT_CODE, or did I miss anything?
According to lxr.php.net, this symbol ZEND_RESULT_CODE is not
referenced in any place except its definition. We can begin using it
if we like, or even rename it. Theoretically renaming it may break
extensions; none that I could find reference it, though.Hey Levi,
I think we should just rename it, it's a little too long. zend_status or ZEND_STATUS would be fine with me, preferably the former since we tend to use lowercase type names. Another possibility might be zend_result.
zend_status +1
If you folks like, I could go ahead and write a patch for master to rename it and use it in a bunch of places.
great, thanks... please only added it to these functions who already
use success/failure, don't change others :)
Here is a preliminary diff for a zend_status:
https://gist.github.com/morrisonlevi/04e9e20f5addadd656c3. It only
changes stuff in Zend/zend_API.{h,c} (except for the zend_status
definition in zend_types.h).
Hey:
There's already ZEND_RESULT_CODE, or did I miss anything?
According to lxr.php.net, this symbol ZEND_RESULT_CODE is not
referenced in any place except its definition. We can begin using it
if we like, or even rename it. Theoretically renaming it may break
extensions; none that I could find reference it, though.Hey Levi,
I think we should just rename it, it's a little too long. zend_status
or ZEND_STATUS would be fine with me, preferably the former since we tend
to use lowercase type names. Another possibility might be zend_result.zend_status +1
If you folks like, I could go ahead and write a patch for master to
rename it and use it in a bunch of places.great, thanks... please only added it to these functions who already
use success/failure, don't change others :)
We could just do the ones returning integer as status as a 2nd step, or in
the same commit. The latter would be cleaner.
Hey:
On Thu, Dec 25, 2014 at 2:33 PM, Michael Wallner mike@php.net
wrote:
There's already ZEND_RESULT_CODE, or did I miss anything?According to lxr.php.net, this symbol ZEND_RESULT_CODE is not
referenced in any place except its definition. We can begin using it
if we like, or even rename it. Theoretically renaming it may break
extensions; none that I could find reference it, though.Hey Levi,
I think we should just rename it, it's a little too long. zend_status
or ZEND_STATUS would be fine with me, preferably the former since we tend
to use lowercase type names. Another possibility might be zend_result.zend_status +1
If you folks like, I could go ahead and write a patch for master to
rename it and use it in a bunch of places.great, thanks... please only added it to these functions who already
use success/failure, don't change others :)We could just do the ones returning integer as status as a 2nd step, or in
the same commit. The latter would be cleaner.
I'm happy about Laruence's idea of unity and consistency.
Having some APIS returns SUCCESS/FAILURE and others return 1/0 is a mess.
+1 for consistency
An enum looks like a good choice.
Julien.P
Hey:
There's already ZEND_RESULT_CODE, or did I miss anything?
yes,
we were talking about use ZEND_RESULT_CODE as return type hinting for
those functions use SUCCSS/FAILURE ..
furthermore, maybe we could use it as all ZEND_API's return type hinting
thanks
Hey:
On Thu, Dec 25, 2014 at 12:38 PM, Pierre Joye pierre.php@gmail.com
wrote:On Wed, Dec 24, 2014 at 4:27 PM, Johannes Schlüter
johannes@schlueters.de wrote:I'm asking for specific things. The reason is that some API's do a
non-zero error code; the fact that they are negative is a detail
that
we should not need to care about.My guess is that positive values more often might have a meaning ("5
items changed", "address 0x1234") whereas negative values less often
have a meaning. Also passing -1 as parameter is more often invalid.
Thus
passing -1 is making debug output look more suspicious.(while there are cases where -1 is valid, see recent famouspid
= fork(); /* ... */ kill(pid, SIGKILL); issue)I don't think this is the same use case as SUCCESS and FAILURE. Many
functions have an out parameter which is only valid when the returned
value is SUCCESS. This is not the same thing as an API which returns
an integer and just happen to embed error state in the negative range.
Notably, it doesn't make sense to dostrpos() == SUCCESSto check
success; these are different cases. My question is specifically
directed at the ones that use SUCCESS and FAILURE: which ones require
FAILURE to be negative instead of the normal UNIX-ism of non-zero?For the record I am in favor of an enum such as
zend_statusor some
other name which indicates whether an operation succeeded or not for
the reasons already cited in this thread. I just don't see why FAILURE
needs to be negative and want to know why this is the case.Hi Levi,
Again, I think the reason FAILURE is -1 is for consistency with other
functions which use negative return values on error. Some functions return
negative error codes, others just -1. Some functions return useful positive
values, others just 0. But the idea is that all functions return a negative
number on error, so you can use if (foo() < 0) to check for errors. That’s
the point of making FAILURE be -1, AIUI. It makes it consistent with other
things, like fork() orstrpos().doing if (foo() < 0 is exactly what should not be done, for any
function returning a status. Only FAILURE and SUCCESS should be used.Which value FAILURE and SUCCESS have is not really relevant here but
to actually be consistent.For example
ZEND_API int zend_hash_del(HashTable *ht, zend_string *key)
should actually be
ZEND_API status zend_hash_del(HashTable *ht, zend_string *key)
and its usage should be:
if (zend_hash_del(ht, key) == FAILURE) {
...
}Same for zend_parse_parameters and the likes.
However functions like zval_update_class_constant
(http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_API.c#1132 ) and all the
underlying functions, are confusing. Both the signature and the return
values should rely on FAILURE/SUCCESS.I think this is what Xinchen means too. Or at least this is what I
mean with unify the APIs.
yes. and as a soft solution.we can change these functions which use success/failure return
zend_status instead of int first.thanks
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
--
Xinchen Hui
@Laruence
http://www.laruence.com/--
--
Xinchen Hui
@Laruence
http://www.laruence.com/