From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8513 invoked by alias); 29 May 2005 08:37:15 -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 8503 invoked by uid 22791); 29 May 2005 08:37:09 -0000 Received: from legolas.inter.net.il (HELO legolas.inter.net.il) (192.114.186.24) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sun, 29 May 2005 08:37:09 +0000 Received: from HOME-C4E4A596F7 (IGLD-80-230-86-142.inter.net.il [80.230.86.142]) by legolas.inter.net.il (MOS 3.5.8-GR) with ESMTP id EMD21242 (AUTH halo1); Sun, 29 May 2005 11:36:17 +0300 (IDT) Date: Sun, 29 May 2005 14:21:00 -0000 Message-Id: From: Eli Zaretskii To: gdb-patches@sourceware.org CC: Christopher Faylor In-reply-to: <20050528234233.GA3440@nevyn.them.org> (message from Daniel Jacobowitz on Sat, 28 May 2005 19:42:34 -0400) Subject: Re: RFA: Use lrealpath instead of gdb_realpath Reply-to: Eli Zaretskii References: <20050528234233.GA3440@nevyn.them.org> X-SW-Source: 2005-05/txt/msg00614.txt.bz2 > Date: Sat, 28 May 2005 19:42:34 -0400 > From: Daniel Jacobowitz > Cc: Christopher Faylor > > Eli, I understand that you think this is a bad choice. However, the mingw32 > maintainers disagree. My only motivation here is to get rid of the > duplicated function As your patch clearly shows, gdb_realpath is _not_ identical to lrealpath. For example, it doesn't have the Windows code that calls GetFullPathName. So we are not replacing a duplicated function, not really. Let me explain my concerns about using lrealpath in its current shape: . The Windows code in lrealpath converts all forward slashes to backslashes, which I think is a bad idea. By contrast, the current gdb_realpath would leave existing forward slashes alone. That is an incompatible change, and my gut feeling is that we should not risk it. On top of that, I think we always use '/' to generate file names, so with this change we will produce ugly file names like D:\foo\bar/baz.c. . The Windows code in lrealpath downcases the file name returned by GetFullPathName. That might be okay for when we need a canonical file name suitable for comparison by strcmp, but in the case of GDB it is IMHO a misservice to the user, since (a) we correctly use FILENAME_CMP to compare file names, and (b) presenting downcased file names to the user is a gratuitous ugliness that we should try to avoid. . GetFullPathName is documented to work even if the path and/or the file name do not exist. By contrast, both realpath and canonicalize_file_name are documented to fail if some or all of the components of the resulting file name do not exist. Thus, I submit that the Windows code is not a faithful emulation of the Posix code, and thus could cause subtle bugs/misfeatures. . lrealpath uses strdup while gdb_realpath uses xstrdup. Thus, if we use lrealpath, we will behave differently when we run out of memory. For these reasons, I believe we should not switch to lrealpath until we address these issues. We could resolve them either by suitable modifications to lrealpath (but then we would need to verify that other users of lrealpath don't suffer; e.g., I suspect that the downcasing part is there because some program uses strcmp to compare file names), or by retaining gdb_realpath with suitable changes to support the MinGW build. > --- symtab.c 8 Mar 2005 04:34:44 -0000 1.145 > +++ symtab.c 28 May 2005 23:13:40 -0000 > @@ -163,7 +163,7 @@ lookup_symtab (const char *name) > { > full_path = xfullpath (name); > make_cleanup (xfree, full_path); > - real_path = gdb_realpath (name); > + real_path = lrealpath (name); > make_cleanup (xfree, real_path); > } Can we please talk about this snippet? I don't understand the need for calling both xfullpath and lrealpath here. If that's because lrealpath might fail if the basename does not exist, I think we should call lrealpath first and fall back on xfullpath only after that.