I just got word of an embarrassing bug in OpenPAM Nummularia. The
is_upper() macro, which is supposed to evaluate to true if its argument is an upper-case letter in the ASCII character set, only evaluates to true for the letter A:
#define is_upper(ch) \ (ch >= 'A' && ch <= 'A')
This macro is never used directly, but it is referenced by
is_letter(), which is referenced by
is_pfcs(), which is used to validate paths and path-like strings, i.e. service names and module names or paths. As a consequence, OpenPAM does not support services or modules which contain an upper-case letter other than A. I never noticed because a) none of the services or modules in use on the systems I use to develop and test OpenPAM have upper-case letters in their names and b) there are no unit or regression tests for the character classification macros, nor for any code path that uses them (except
openpam_readword(), which uses
The obvious course of action is to add unit tests for the character classification macros (r760) and then fix the bug (r761). In this case, complete coverage is easy to achieve since there are only 256 possible inputs for each predicate.
I have merged the fix to FreeBSD head (r262529 and r262530). Impatient users can fix their system by running the following commands:
% cd /usr/src/contrib/openpam % svn diff -r758:762 svn://svn.openpam.org/openpam/trunk | patch % cd /usr/src/lib/libpam/libpam % make && make install
Unsurprisingly, writing more unit tests for OpenPAM is moving up on my TODO list. Please contact me if you have the time and inclination to help out.
2 thoughts on “On testing, part III”
Why not just use isupper() from ctype.h?
They are not equivalent. The question I need answered is “is this character an upper-case letter in the ASCII character set”, not “is this character an upper-case letter in the current locale”.