Hello Bruno, thanks for excellent review. When the function's behavior is definitely accepted by people here, I will improve its implementation.
On Wednesday 17 September 2008 13:13:03 you wrote: > There are still a few things to do before we can add it to gnulib: > - Do you have a copyright assignment for contributions to gnulib on file? > - Under which copyright is the original verrevcmp code? If it is not > owned by the FSF, we can quickly redevelop it, from a formal description of > this function. There was something like "Copyright (C) 1995 Ian Jackson" in original file, but also "... under terms of GNU General Public License..." I am not lawyer and don't understand such details :-) > - Write a .h file that declares the function and describes what it does. > - Write a module description. > - Fix the obvious bugs, for example, you need to cast 'char' values to > 'unsigned char' before passing them to <ctype.h> functions. Sure. > - 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)? Good idea. I just wanted to separate original code from dpkg and the new code to clarify what is changed and why. Once it will be accepted, I can change its implementation and run some regression tests on it. Maybe also compare its performance with glibc (not working) solution... > - Remove unnecessary casts, for example, if match_suffix took 'char *' > arguments and returned a 'char *', no casts would be necessary. Well, this is cosmetic detail. I am not sure about benefit of such change. If you let it declared 'const char *', compiler will check for any attempts to write to input within match_suffix(). Note this casts will disappear when it will use 4 arguments (str1, len1, str2, len2). Thanks for advice. > - GNU coding style: space between function name and opening parenthesis, > write 'const char *', not 'const char*', etc. > - Put the unit test in the usual form used by gnulib. See the module > 'printf-posix-tests' for a test that produces some textual output and > compares it to an expected output. Sure. So now the main question is if the behavior of this function is successful in most use cases or not. I'd also like to know your opinion if it is possible to change 'ls -v' to use this function or not. Kamil
