RE: [PATCH v1] gpudev: return EINVAL if invalid input pointer for free and unregister
From: Morten Brørup <hidden>
Date: 2021-12-03 10:37:15
From: Morten Brørup [mailto:mb@smartsharesystems.com] Sent: Thursday, 2 December 2021 14.56quoted
From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com] Sent: Thursday, 2 December 2021 14.01quoted
quoted
From: Thomas Monjalon [mailto:thomas@monjalon.net] Sent: Thursday, 2 December 2021 08.19 01/12/2021 22:37, Tyler Retzlaff:quoted
On Wed, Nov 24, 2021 at 06:04:56PM +0000, Bruce Richardsonwrote:quoted
quoted
quoted
quoted
quoted
if (ret < 0 && rte_errno == EAGAIN)i only urge that this be explicit as opposed to a range i.e.retquoted
== -quoted
quoted
1quoted
preferred over ret < 0I don't understand why you think it is important to limit returnvaluequoted
quoted
to -1. Why "if (ret == -1)" is better than "if (ret < 0)" ?Speaking for myself: For clarity. It leaves no doubt that "it failed" is represented bythe return value -1, and that the function does not return errnovaluesquoted
such asquoted
-EINVAL.But why '< 0' gives you less clarity? Negative value means failure - seems perfectly clear to me.I disagree: Negative value does not mean failure. Only -1 means failure. There is no -2 return value. There is no -EINVAL return value. Testing for (ret < 0) might confuse someone to think that other values than -1 could be returned as indication of failure, which is not the case when following the convention where the functions set errno and return -1 in case of failure. It would be different if following a convention where the functions return -errno in case of failure. In this case, testing (ret < 0) would be appropriate. So explicitly testing (ret == -1) clarifies which of the two conventions are relevant.
I tested it on Godbolt, and (ret < 0) produces slightly smaller code than (ret == -1) on x86-64: https://godbolt.org/z/3xME3jxq8 A binary test (Error or Data) uses 1 byte less, and a tristate test (Error, Zero or Data) uses 3 byte less. Although there is no measurable performance difference for a single instance of this kind of test, we should consider that this kind of test appears many times in the code, so the saved bytes might add up to something slightly significant in the instruction cache. My opinion is not so strong anymore... perhaps we should prefer performance over code readability, also in this case?