From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6040 invoked by alias); 2 Apr 2002 15:22:42 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6030 invoked from network); 2 Apr 2002 15:22:40 -0000 Received: from unknown (HELO dublin.ACT-Europe.FR) (212.157.227.154) by sources.redhat.com with SMTP; 2 Apr 2002 15:22:40 -0000 Received: from berlin.ACT-Europe.FR (berlin.int.act-europe.fr [10.10.0.169]) by dublin.ACT-Europe.FR (Postfix) with ESMTP id 6FF08229E27; Tue, 2 Apr 2002 17:22:39 +0200 (MET DST) Received: by berlin.ACT-Europe.FR (Postfix, from userid 507) id 1A27E960; Tue, 2 Apr 2002 17:22:33 +0200 (CEST) Date: Tue, 02 Apr 2002 07:22:00 -0000 From: Joel Brobecker To: Eli Zaretskii Cc: gdb-patches@sources.redhat.com Subject: Re: [RFC] xfullpath and new regression test xfullpath.exp Message-ID: <20020402172232.C25687@act-europe.fr> References: <20020401105021.A13168@act-europe.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from eliz@is.elta.co.il on Mon, Apr 01, 2002 at 12:42:13PM +0200 X-SW-Source: 2002-04/txt/msg00022.txt.bz2 > Thanks. However, this patch will not DTRT on DOS and Windows systems, > I'm afraid. Details below. I'm quite ignorant of the DOS and Windows platforms, so I'm not too surprised, somehow. I thought that since only cygwin-gdb was supported, I did not have to deal with DOS-like formats. Thanks for correcting me. > This kind of detailed description should go into the source, IMHO. It's > okay to have it in the ChangeLog as well, but having it _only_ in the > ChangeLog is not a good idea: people who read the code don't expect to > find the associated comments in the logs. Right, I will put this in the code as well. > > + /* > + * xfullpath > + * > + * Return a copy of FILENAME, with its directory prefix canonicalized > + * by gdb_realpath. > + */ > > I believe this is not the comment style the GNU project uses. Is the following style more in line with the GNU project style? /* Return a copy of FILENAME, with its directory prefix canonicalized by gdb_realpath. */ > This doesn't work correctly when the input name is something like > "d:foo". lbasename returns a pointer to `f', so you end up with the > directory being "d:", and the rest of the code will generate "d:/foo", > which is something very different in semantics. The Right Thing to do > in this case is to return either the unmodified "d:foo" or "d:./foo". Ah, Ok, I did not know this, I thought that d:foo and d:/foo were equivalent. Shouldn't xfullpath return d:/foo instead? What do you think of the following pseudo-code? [...] dir_name = alloca ((size_t) (base_name - filename + 2)); /* Allocate enough space to store the dir_name + plus one extra character sometimes needed under Windows (see below), and then the closing \000 character */ strncpy (dir_name, filename, base_name - filename); dir_name[base_name - filename] = '\000'; #if defined(__CYGWIN__) || defined(__MINGW32__) if (strlen (dir_name) == 2 && is_letter (dir_name[0]) && dir_name[1] == ':') { dir_name[2] = '.'; dir_name[3] = '\000'; } #endif real_path = gdb_realpath (dir_name); [...] > This doesn't work if the input is "\\", which is also a root directory. > You should use the more portable IS_DIR_SEPARATOR macro (defined on > include/filenames.h) instead. Cool macro. > Finally, I don't understand why do you avoid duplicate slashes when > dir_name is "/", but not if it is "/foo/bar/". Why not handle the > other case as well? (This issue is a general one, not something specific > to MS-Windows, although the case of "d:/", the root directory on drive d:, > was what initially brought that to my attention.) Since I thought that only cygwin-style filenames were supported, I thought that the the double-slash issue would only happen with the root directory. Is this better if I modify the last part of the function to be (can I consider that SLASH_STRING will always be one character long?): if (IS_DIR_SEPARATOR (real_path[strlen (real_path) - 1])) result = concat (real_path, base_name, NULL); else result = concat (real_path, SLASH_STRING, base_name, NULL); Thanks, -- Joel