On 02/18/2011 11:14 AM, Igor Galić wrote:
I accidentally sent this one only to Leif,
here it is for the whole list:
So, I've finished most of this work, and while reading code, and
discussing on IRC, I think we'll need to make one more change. The
issue
is that a few APIs uses an "int" return code as a boolean, where 1
means
"success" (e.g. you found a header), and 0 means "failure" (e.g.
not
found). I'd like to propose that we change these to use
TSReturnCode
as
well (TS_SUCCESS and TS_ERROR).
I'm much rather see a boolean, or a TSBool, because stdbool.h is
not supported with GCC 4.2.1 (the minimum we require)
The only question that leaves me with is: Consistency.
The downside to this is that such APIs now requires code changes in
any
existing plugins. It's not impossible to find such cases, but a
little
problematic (since TS_SUCCESS == 0, the logic is inversed). I think
I
can whip up a Perl script that would produce good warnings on areas
in
existing plugin code that the author might need to address the API
changes. A few example APIs that would change include
TSMgmtStringGet()
TSIPLookupMatchFirst()
How can we see, looking at the function call alone, this
is expected to return a boolean, rather than TSReturnCode?
See if the conditional is comparing against the enum value TS_ERROR or
TS_SUCCESS. On assignment you can check to see if it is assigning to an
enum type of TSReturnCode.
There might be some false positives on some of the checks, such as
defining an enum type one place in the code and assigning to it later in
the code. Also, some people might want to be lazy and assume that
TS_SUCCESS is always going to be 0 and not use the enum value in a
conditional.
Are there any objections or other concerns about this change? It
should
be notable that there are plenty of other API changes, but in most
cases, the compilers will detect / complain on those. For this, it
would not.
With the above in mind, going for consistency is probably better.
-- Leif
i
