From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23852 invoked by alias); 27 Jan 2012 12:22:24 -0000 Received: (qmail 23838 invoked by uid 22791); 27 Jan 2012 12:22:20 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_SM,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 27 Jan 2012 12:22:02 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0RCM05k001861 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 27 Jan 2012 07:22:00 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0RCLxY5022573; Fri, 27 Jan 2012 07:21:59 -0500 Message-ID: <4F2296E7.1090705@redhat.com> Date: Fri, 27 Jan 2012 12:36:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0 MIME-Version: 1.0 To: Eli Zaretskii CC: asmwarrior@gmail.com, gdb-patches@sourceware.org Subject: Re: fix build error on MinGW (HAVE_READLINK) undefined References: <4F2206DE.20907@gmail.com> <83k44dzejs.fsf@gnu.org> <4F22760A.2020309@redhat.com> <83ehulz77q.fsf@gnu.org> In-Reply-To: <83ehulz77q.fsf@gnu.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: 2012-01/txt/msg00945.txt.bz2 On 01/27/2012 11:59 AM, Eli Zaretskii wrote: >> Date: Fri, 27 Jan 2012 10:01:46 +0000 >> From: Pedro Alves >> CC: asmwarrior , gdb-patches@sourceware.org >> >>> I think you need to set errno to EINVAL in the #else branch, because >>> the meaning of that error on systems that do have readlink is "the >>> named file is not a symbolic link". >>> >>> On second thought, perhaps a better way would be to define a readlink >>> for MinGW that always sets errno to EINVAL and returns -1. Then the >>> ugly #ifdef can go away. >> >> The corresponding native side returns ENOSYS/FILEIO_ENOSYS, indicating >> the function is not supported by the implementation. GDBserver should do >> the same. > > But isn't ENOSYS sub-optimal in this case? Systems that don't have > readlink don't have symlinks, either. Not necessarily true. Might have symlinks but not a defined MAXPATHLEN, for example (I guess the Hurd may fall on that basked). Might have symlinks, but need some other mechanism or system call to read them. The error is really "the `readlink' operation is not supported", so ENOSYS is the best fit. > So any file there is trivially, > by definition, not a symlink. Why not tell that to the caller, and > have the rest of the code work "normally"? Because it isn't what happened. You'd still confuse the client side. E.g., if the file does not exist, we should return ENOENT instead. > By contrast, setting errno > to ENOSYS will most likely be processed as an exceptional situation, > like failing the command that invoked readlink. How is that TRT? The code on the client that invoked readlink should cope. There's no such thing currently, but if there was a generic command that invoked a readlink on a Windows target, and it got ENOSYS, it would then say "operation not supported" if it chose, or fallback to some other means. Returning "not a symlink" could confuse the client. > >>>> static char * >>>> inf_child_fileio_readlink (const char *filename, int *target_errno) >>>> { >>>> /* We support readlink only on systems that also provide a compile-time >>>> maximum path length (MAXPATHLEN), at least for now. */ >>>> #if defined (HAVE_READLINK) && defined (MAXPATHLEN) >>>> char buf[MAXPATHLEN]; >>>> int len; >>>> char *ret; >>>> >>>> len = readlink (filename, buf, sizeof buf); >>> >>> Here too. >> >> The #else branch just below reads: >> >> #else >> *target_errno = FILEIO_ENOSYS; >> return NULL; >> #endif > > Which is wrong, don't you think? I don't. -- Pedro Alves