Feature #14601
open
want 3-argument string comparison assertions
0%
Description
ASSERT3x() and VERIFY3x() are valuable debugging tools, but they are not useful replacements for ASSERT(strcmp(x, y) 0). I count 24 places where kernel callers make such an assertion; 23 of the 24 could use an ASSERT3STR() in place of the assertion. The last case asserts that the string to be compared matches either of two constants which, like ASSERT(x 4 || x == 5), cannot be replaced this way.
There are two ways we could do this, trading off consistency against risk of confusion:
ASSERT3STR(str1, ==, str2);
This would test strcmp(str1, str2) against 0. Callers wishing to assert that the pointers are the same would use the existing ASSERT3P, but there is some small risk that the distinction would not be obvious. Instead we could do:
ASSERTSTREQ(str1, str2); ASSERTSTRNE(str1, str2);
which is inconsistent with the other syntax and requires two macros in place of a more flexible one, but does make it much less likely that anyone would expect this to be testing for pointer equality. I prefer the first syntax (with its partner VERIFY3STR()) but either is better than none.
Updated by Dan McDonald 3 months ago
What if some grinning weirdo expects, for example:
ASSERT3STR(str1, >, str2)
to work? Maybe macro-eval-time checking ("Don't do that!") alleviates my concern, however. (I'm assuming ASSERT3STR could take "!=" as well?)
Updated by Thirteen Oxide 3 months ago
Dan McDonald wrote in #note-1:
What if some grinning weirdo expects, for example:
[...]
to work? Maybe macro-eval-time checking ("Don't do that!") alleviates my concern, however. (I'm assuming ASSERT3STR could take "!=" as well?)
That would expand to, morally,
ASSERT(strcmp(str1, str2) > 0);
Or to be a bit clearer:
extern void assfail3str(const char *, const char *, const char *, const char *, const char *, int);
#define VERIFY3STR_IMPL(LEFT, OP, RIGHT, TYPE, FUNC) do { \
const TYPE __left = (TYPE)(LEFT); \
const TYPE __right = (TYPE)(RIGHT); \
if (!((FUNC)(__left, __right) OP 0)) { \
assfail3str(#LEFT " " #OP " " #RIGHT, \
__left, #OP, __right, \
__FILE__, __LINE__); \
_NOTE(CONSTCOND) } while (0)
#define VERIFY3STR(x, y, z) VERIFY3STR_IMPL(x, y, z, char *, strcmp)
This isn't quite ideal but without changing the existing assfail3 (which we must not do) to take another argument indicating how to interpret __left and __right this is the best I could come up with.
Currently there are no callers doing anything other than == or != here, but the effect of this would seem consistent with my expectations: it asserts that the relationship between the strings is the one expressed by the operator argument. What would you expect here? What would you want?
Updated by Dan McDonald 3 months ago
What would you expect here? What would you want?
I had no idea what to expect here, but your implementation detail answers well-enough my first question (it checks the comparison function return), and prompts me to ask a second question:
Do we wish to use strcmp(), or strncmp() with a reasonably high (but constant) limit? I'm thinking this might be a case where we let users potentially shoot themselves in the foot with strcmp(), but if we do, it should at least be documented?