Hi, On Wed, 18 Feb 2009, Johannes Sixt wrote: > Johannes Schindelin schrieb: > > The function strip_path_suffix() will try to split the given path into > > prefix/suffix. The suffix has to be passed to the function, and if > > the path matches, the prefix is set. > > > > Arbitrary runs of directory separators ("slashes") are assumed > > identical. > > > > Example: > > > > strip_path_suffix("C:\\msysgit/\\libexec\\git-core", > > "libexec///git-core", &prefix) > > > > will set prefix to "C:\\msysgit" and return 0. > > But you implemented it so that prefix is actually "C:\\msysgit/\\" > (unless, of course, I'm reading the code wrong). This is supposed to handle that case: *prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len)); > > +/* sets prefix if the suffix matches */ > > +int strip_path_suffix(const char *path, const char *suffix, const char > > **prefix) > > For a general purpose function, the API is very unnatural (and geared > towards its only user in patch 2/2). I'd expect that the return value is > the result or NULL on failure. Good point. Will change. > > +{ > > + int path_len = strlen(path), suffix_len = strlen(suffix); > > + > > + while (suffix_len) { > > + if (!path_len) > > + return 1; > > + > > + if (is_dir_sep(path[path_len - 1])) { > > + if (!is_dir_sep(suffix[suffix_len - 1])) > > + return 1; > > + path_len = chomp_trailing_dir_sep(path, path_len); > > + suffix_len = chomp_trailing_dir_sep(suffix, > > suffix_len); > > + } > > + else if (path[--path_len] != suffix[--suffix_len]) > > + return 1; > > + } > > + > > + if (path_len && !is_dir_sep(path[path_len - 1])) > > + return 1; > > Should strip_path_suffix("foo/bar", "foo/bar", &prefix) succeed and > prefix be the empty string? This implementation says it should be so. > That's just a question... Yes, I assumed that that makes sense. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html