On Wednesday 17 September 2008 13:13:03 you wrote: > - Fix the obvious bugs, for example, you need to cast 'char' values to > 'unsigned char' before passing them to <ctype.h> functions. > - Streamline the operation: Can't you get rid of copying the two strings? > For example, by changing verrevcmp to take 4 arguments > (str1, len1, str2, len2) instead of NUL terminated arguments (str1, > str2)? > - Remove unnecessary casts, for example, if match_suffix took 'char *' > arguments and returned a 'char *', no casts would be necessary. > - GNU coding style: space between function name and opening parenthesis, > write 'const char *', not 'const char*', etc New version of filevercmp.c is attached, now without copying of the input strings. I've also made a simple performance test - the speed was about 7-8x slower than original glibc strverscmp function, but the glibc's result is mostly wrong.
Kamil
/* Copyright (C) 1988, 1991-2008 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <config.h> #include "system.h" #include "filevercmp.h" #define xisalnum(c) isalnum ((unsigned char) c) #define xisdigit(c) isdigit ((unsigned char) c) #define xisalpha(c) isalpha ((unsigned char) c) int verrevcmp (const char *s1, size_t s1_len, const char *s2, size_t s2_len); const char * match_suffix (const char **str); /* proposed new function to compare version strings (and files with version) This function is supposed to be replacement for STRVERSCMP function. Same parameters and return value as standard STRCMP function. It is based on verrevcmp function from dpkg and improved to deal better with file suffixes. */ int filevercmp (const char *s1, const char *s2) { const char *s1_pos, *s2_pos; const char *s1_suffix, *s2_suffix; int s1_len, s2_len; /* easy comparison to see if versions are identical */ if (!strcmp (s1, s2)) return 0; /* "cut" file suffixes */ s1_pos = s1; s2_pos = s2; s1_suffix = match_suffix (&s1_pos); s2_suffix = match_suffix (&s2_pos); s1_len = (s1_suffix ? s1_suffix : s1_pos) - s1; s2_len = (s2_suffix ? s2_suffix : s2_pos) - s2; /* restore file suffixes if strings are identical after "cut" */ if ((s1_suffix || s2_suffix) && (s1_len == s2_len) && 0 == strncmp (s1, s2, s1_len)) { s1_len = s1_pos - s1; s2_len = s2_pos - s2; } return verrevcmp (s1, s1_len, s2, s2_len); } /* match file suffix defined as RE (\.[A-Za-z][A-Za-z0-9]*)*$ Scan string pointed by *str and return pointer to suffix begin or NULL if not found. Pointer *str points to ending zero of scanned string after return. */ const char * match_suffix (const char **str) { const char *match = NULL; bool read_alpha = false; while (**str) { if (read_alpha) { read_alpha = false; if (!xisalpha (**str)) match = NULL; } else if ('.'==**str) { read_alpha = true; if (!match) match = *str; } else if (!xisalnum (**str)) match = NULL; (*str)++; } return match; } /* verrevcmp helper function */ inline int order (unsigned char c) { if (c == '~') return -1; else if (xisdigit (c)) return 0; else if (xisalpha (c)) return c; else return c + 256; } /* slightly modified ververcmp function from dpkg S1, S2 - compared string S1_LEN, S2_LEN - length of strings to be scanned */ int verrevcmp (const char *s1, size_t s1_len, const char *s2, size_t s2_len) { int s1_pos = 0; int s2_pos = 0; while (s1_pos < s1_len || s2_pos < s2_len) { int first_diff= 0; while ((s1_pos < s1_len && !xisdigit (s1[s1_pos])) || (s2_pos < s2_len && !xisdigit (s2[s2_pos]))) { int s1_c = (s1_pos == s1_len) ? 0 : order (s1[s1_pos]); int s2_c = (s2_pos == s2_len) ? 0 : order (s2[s2_pos]); if (s1_c != s2_c) return s1_c - s2_c; s1_pos++; s2_pos++; } while (s1[s1_pos] == '0') s1_pos++; while (s2[s2_pos] == '0') s2_pos++; while (xisdigit (s1[s1_pos]) && xisdigit (s2[s2_pos])) { if (!first_diff) first_diff = s1[s1_pos] - s2[s2_pos]; s1_pos++; s2_pos++; } if (xisdigit (s1[s1_pos])) return 1; if (xisdigit (s2[s2_pos])) return -1; if (first_diff) return first_diff; } return 0; }
