- changed status to open
- removed comment
CCTK_RegexMatch does not return a distinguishable error condition when the regular expression is invalid
CCTK_RegexMatch right now returns 1 if the pattern matches the string and 0 if either the pattern does not match or could not be compiled via regcomp. It would be useful if user code could distinguish between these two cases. The attached patch changes the return value in the "does not compile" case to -1 and updates all source files that I could find that use it.
Note that this patch changes behaviour of a routine. It used to return 0 for non-compiling patterns so thorns that test for C-like true would interpret invalid patterns as does-not-match, but will interpret the -1 return value as does-match.
Keyword:
Comments (7)
-
reporter -
- removed comment
A regular expression that does not compile indicates a serious error, one that presumably currently does not exist. I suggest adding a CCTK_WARN (e.g. level 1) to help spot these problems.
-
reporter - removed comment
I updated the patch to output a ALERT level warning in the routines calling RegexMatch. It's actualy rather hard to actually trigger it though right now. The only callers of RegexMatc hare the routines that validate parameter ranges. Since the perl configuration scripts already check regular expressions for correctness one has to construct a valid perl regex that fails in C (the perl extended regex `(?|.*)` does the trick).
-
- changed status to open
- removed comment
Please apply.
-
reporter - removed comment
I found I had forgotten one place where RegexMatch was called (with a hardcoded pattern, so no real need to check for invalid patterns). I have also added documentation to the reference manual. Finally the protype for CCTK_RegexMatch was moved to Misc.h so that RegexMatch would become a user visible function rather than a function used only internally (it was never CCTKi_RegexMatch though).
Still ok to apply?
-
- removed comment
Yes, even hardcoded patterns need to be checked, because even they can change (in editors).
Yes, still okay to apply.
-
reporter - changed status to resolved
- removed comment
applied as rev 4831 of the flesh.
- Log in to comment