Verify error codes inconsistent with rest of flatcc error codes

[gtaska]

This may be a noob trap that I fell into, but I believe that things could be improved.

All the flatccverify* functions return an int value, just like the builder API, however error codes are represented differently.

It was non-trivial to determine that the int values returned by flatccverify* functions are of type enum flatcc_verify_error_no, and unlike the builder API, the error codes are positive values, not negative, and do not work with flatbuffers_failed(). (In hindsight, I see that this is defined in flatbuffers_common_builder.h, so I guess there shouldn't be any expectation that this works with other portions of the flatcc API, but this is a subtlety that is easily lost when you're trying to learn the flatcc API from scratch)

Suggestions:

(any of these would help)

• Have the verify functions return a flatcc_verify_error_no, rather than int to make it obvious that these are not the same.
• Modify the enum flatcc_verify_error_no so the error values are negative, consistent with the builder API
• Clearly document in the flatccverify* documentation that 0 is success and any non-zero value is a failure.
• Improve the example in the main README.md. At the moment, the expected behaviour can only be inferred from the monster example, however this is complicated by the needless assignment within the if condition check, making it more difficult to read than it needs to be. IMHO, the readability of, and information conveyed by this example could be greatly improved by re-writing the sample code to:

... finalize
ret = ns(Monster_verify_as_root_with_identifier(buffer, size, "MONS"))
if (ret != flatcc_verify_ok) {
    printf("Monster buffer is invalid: %s\n",
    flatcc_verify_error_string(ret));
}

[mikkelfj]

Thanks for the input. Things can always be improved, but at this point it would be an unnecesary breaking change to change the sign of the return value.

There is precedence for this behaviour: The C errno.h error codes are typically positive and 0 on error. Negative return codes are primarily used if a positive result is generally returned such as the length of data being read, or if there is a boolean error code where 0 means success and -1 means there was some error, please check the system error code.

In this case the function that actually provides error info returns a positive error code where as normal operation either returns 0 or -1, or a pointer or null.

Therefore, as I see it, the system is not very surprising from a C convention although it can be argued that the C convention could be improved.

Maybe an enum as return value would be clearer, but again, there is precedence for using an integer in this context.

This leaves you suggestions for improving the documentation and examples. I am open to pull requests for that, as long as they are mindful to the design choices made in the code.

Note that there is already a documentation section that warns about the pitfall of null return values as error vs. -1 for other calls. This is annoying but again conventional - new convention is to always return an integer error status and provide any return value via a pointer argument. I try to do that in some software I develop, but it isn't always practical.

[mikkelfj]

I am re-reading your original post. I think I was a bit hasty to respond. I was thinking about a different context such as the JSON parser and parser result code.

The verify functions can indeed be confusing as you point. To follow convention they should return -1 and then another function should retrieve the positive error code. However, the logic behind this, I think, is to avoid this extra step because the error code and the result of the operation is the same thing. Therefore there needed to be a tradeoff between extra complexity and conformance and I chose to reduce complexity. I generally use positive codes when the error code carry information, as is convention. Thus the descrepancy follows from that.

The documentation can probably still be improved as you suggest, including the motivation as to why there is this difference.