> Date: Tue, 8 Mar 2011 11:56:45 +0100 > From: Kai Tietz <ktiet...@googlemail.com> > > +@deftypefn Extension int filename_dirchr (const char *@var{p}) > + > +The returned value is similar to what @code{strchr} would return for > +searching for a directory separator. > + > +This function does not normalize file name. However, it does handle > +the fact that on DOS-like file systems, forward and backward slashes > +are directory separators.
This is very mysterious. The documentation should explain how this is "handled", or else the user will have no choice but to look in the sources. And description "by similarity" doesn't help, because this function is obviously different from strchr in _some_ ways, but you don't say how. While at that, explain the problem this solves, or else the raison d'etre of this function will not be understood. We do want this function to be used instead of just strchr, don't we? For it to be used, its purpose and advantages should be well understood. Btw, why do we need filename_dirchr? The use case for filename_dirrchr is clear, but in what situations will we need the other one? > + if (!r || (r2 && r2 < r)) Why do you test for r2 being non-NULL? You are not going to dereference it in the next comparison, and NULL is comparable as any other value. > +@deftypefn Extension int filename_dirrchr (const char *@var{p}) > + > +The returned value is similar to what @code{strrchr} would return for > +searching for a directory separator. > + > +This function does not normalize file name. However, it does handle > +the fact that on DOS-like file systems, forward and backward slashes > +are directory separators. Same comments about this doc. > + if (!r || (r2 && r2 > r)) And same comment here about testing r2 for non-NULL value. Please also wait for others to review, as I'm not authorized to approve the changes. Thanks for working on this.