From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2475 invoked by alias); 8 Mar 2011 11:12:10 -0000 Received: (qmail 2450 invoked by uid 22791); 8 Mar 2011 11:12:08 -0000 X-SWARE-Spam-Status: No, hits=-0.7 required=5.0 tests=AWL,BAYES_50,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Mar 2011 11:12:01 +0000 Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1Pwupb-0001ns-M8; Tue, 08 Mar 2011 06:11:59 -0500 Date: Tue, 08 Mar 2011 11:53:00 -0000 Message-Id: From: Eli Zaretskii To: Kai Tietz CC: binutils@sourceware.org, gdb-patches@sourceware.org, gcc-patches@gcc.gnu.org In-reply-to: (message from Kai Tietz on Tue, 8 Mar 2011 11:56:45 +0100) Subject: Re: [patch libiberty include]: Add additional helper functions for directory-separator searching Reply-to: Eli Zaretskii References: X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg00526.txt.bz2 > Date: Tue, 8 Mar 2011 11:56:45 +0100 > From: Kai Tietz > > +@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.