On Thu, Dec 8, 2011 at 12:27 PM, David Sommerseth <openvpn.l...@topphemmelig.net> wrote: > Hi Alon, > > I'm not sure I understand the critique too well now. As this is the > approach I tried to do. However, I put the basename() and dirname() > implementations in compat.h, calling the combined openvpn_dirbasename() > function. > > The reason for the combined version is basically to keep as much generic > code as possible. And the old openvpn_basename() has been tested and > tried for many years, so that code base can be considered solid. As you > know, the difference between dirname() and basename() is basically if you > return the string from the beginning to the split point, or from the > split point and to the end of the string. > > But I can move the static inline functions from the compat.h file over to > the compat.c file, and declare openvpn_dirbasename() static instead. If > that is a better solution. >
Yes, this what I meant... :) Best not to put code in headers, unless absolutely required... when? if you prove by profiler that you can optimize anything... Most likely you won't call basename in 1000000000 loop. However, you should be very careful from relying of the "solid" openvpn_dirbasename as it won't be used at most systems... as most have basename/dirname... This is the code from glibc, which most probably be linked into your code, the only difference in windows it to check both '/' and '\'.... I like to use '/' also in windows. But if you don't like it we can do AC_DEFINE([PATH_SEPARATOR], ...) in configure.ac, if not already there, then use PATH_SEPARATOR constant in code. --- char * basename (filename) const char *filename; { char *p = strrchr (filename, '/'); return p ? p + 1 : (char *) filename; } char * dirname (char *path) { static const char dot[] = "."; char *last_slash; /* Find last '/'. */ last_slash = path != NULL ? strrchr (path, '/') : NULL; if (last_slash != NULL && last_slash != path && last_slash[1] == '\0') { /* Determine whether all remaining characters are slashes. */ char *runp; for (runp = last_slash; runp != path; --runp) if (runp[-1] != '/') break; /* The '/' is the last character, we have to look further. */ if (runp != path) last_slash = __memrchr (path, '/', runp - path); } if (last_slash != NULL) { /* Determine whether all remaining characters are slashes. */ char *runp; for (runp = last_slash; runp != path; --runp) if (runp[-1] != '/') break; /* Terminate the path. */ if (runp == path) { /* The last slash is the first character in the string. We have to return "/". As a special case we have to return "//" if there are exactly two slashes at the beginning of the string. See XBD 4.10 Path Name Resolution for more information. */ if (last_slash == path + 1) ++last_slash; else last_slash = path + 1; } else last_slash = runp; last_slash[0] = '\0'; } else /* This assignment is ill-designed but the XPG specs require to return a string containing "." in any case no directory part is found and so a static and constant string is required. */ path = (char *) dot; return path; } --- Alon.