I'm not sure if and when the function
Code:
int isname(const char *str, const char *namelist)
in handler.c is ever called to compare numerical strings, but there are
two problems with an edit made by someone named Sryth. As indicated by their comment, the intention was to prevent the function from allowing abbreviations.
1) This has introduced a (benign) memory leak, since the string newlist is not freed in one of the cases.
2) But far more importantly, this does not work as intended. Using the function as it is currently written,
Code:
isname("123", "123 234 345") <--- evaluates to TRUE, as intended
isname("123", "1234 2345") <--- evaluates to FALSE, as intended
isname("123", "1234 123") <--- evaluates to FALSE, which could not have been intended
The reason the 3rd statement evaluates to FALSE, is that since "123" is a proper substring of "1234", but has a leading character which is numeric, the function immediately returns FALSE, without allowing "123" to be compared to the next token in in the namelist (which is in fact a perfect match).
This simplest fix is the following:
Code:
/* Don't allow abbreviated numbers. - Sryth */
if (isdigit(*str) && (atoi(str) != atoi(curtok)))
- return 0;
+ continue;
free(newlist);
return 1;
However, I would instead recommend the following change (which removes more lines than it adds):
Code:
--- handler.c.old 2020-02-21 18:11:19.000000000 -0700
+++ handler.c 2020-02-21 18:48:56.000000000 -0700
@@ -85,24 +85,22 @@ int isname(const char *str, const char *
{
char *newlist;
char *curtok;
+ int ret_val = FALSE;
if (!str || !*str || !namelist || !*namelist)
- return 0;
-
- if (!strcmp(str, namelist)) /* the easy way */
- return 1;
+ return FALSE;
newlist = strdup(namelist); /* make a copy since strtok 'modifies' strings */
for(curtok = strtok(newlist, WHITESPACE); curtok; curtok = strtok(NULL, WHITESPACE))
if(curtok && is_abbrev(str, curtok)) {
- /* Don't allow abbreviated numbers. - Sryth */
+ /* Don't allow abbreviated numbers */
if (isdigit(*str) && (atoi(str) != atoi(curtok)))
- return 0;
- free(newlist);
- return 1;
+ continue;
+ ret_val = TRUE;
+ break;
}
free(newlist);
- return 0;
+ return ret_val;
}
Again, it probably never would have caused an issue, but the code might as well be correct.
