From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19130 invoked by alias); 10 Jun 2009 15:14:41 -0000 Received: (qmail 19114 invoked by uid 22791); 10 Jun 2009 15:14:40 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-ew0-f228.google.com (HELO mail-ew0-f228.google.com) (209.85.219.228) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 10 Jun 2009 15:14:35 +0000 Received: by ewy28 with SMTP id 28so514001ewy.24 for ; Wed, 10 Jun 2009 08:14:32 -0700 (PDT) Received: by 10.210.120.7 with SMTP id s7mr1797795ebc.20.1244646870848; Wed, 10 Jun 2009 08:14:30 -0700 (PDT) Received: from ?192.168.2.99? (cpc2-cmbg8-0-0-cust61.cmbg.cable.ntl.com [82.6.108.62]) by mx.google.com with ESMTPS id 24sm144735eyx.13.2009.06.10.08.14.28 (version=SSLv3 cipher=RC4-MD5); Wed, 10 Jun 2009 08:14:29 -0700 (PDT) Message-ID: <4A2FD0A7.2090408@gmail.com> Date: Wed, 10 Jun 2009 15:14:00 -0000 From: Dave Korn User-Agent: Thunderbird 2.0.0.17 (Windows/20080914) MIME-Version: 1.0 To: Paul Pluzhnikov CC: binutils@sourceware.org, Tom Tromey , gdb-patches ml , Dave Korn Subject: Re: How to get file descriptor from abfd? References: <8ac60eac0905311056l3b8edf98rc6abfe28853e0b6d@mail.gmail.com> <4A22CB56.3070704@gmail.com> <8ac60eac0906012254o57756790y9bb4eb23d1d6e5a1@mail.gmail.com> <8ac60eac0906080910x6497e1abg5c7aa9bdf863c5d5@mail.gmail.com> In-Reply-To: <8ac60eac0906080910x6497e1abg5c7aa9bdf863c5d5@mail.gmail.com> 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: 2009-06/txt/msg00255.txt.bz2 Paul Pluzhnikov wrote: > On Mon, Jun 1, 2009 at 10:54 PM, Paul Pluzhnikov wrote: > >> Would attached patch be acceptable? > > Ping? I can't approve this as it's outside my area. I have verified that it applies cleanly and builds without failures on i686-pc-cygwin (although that doesn't support --enable-mmap currently for reasons I have yet to discover) and have just a couple of minor formatting comments on the patch: > Index: bfdio.c > =================================================================== > RCS file: /cvs/src/src/bfd/bfdio.c,v > retrieving revision 1.21 > diff -u -p -u -r1.21 bfdio.c > --- bfdio.c 24 May 2009 11:47:27 -0000 1.21 > +++ bfdio.c 2 Jun 2009 05:30:15 -0000 > @@ -158,6 +158,8 @@ DESCRIPTION > . int (*bclose) (struct bfd *abfd); > . int (*bflush) (struct bfd *abfd); > . int (*bstat) (struct bfd *abfd, struct stat *sb); > +. void (*bmmap) (struct bfd *abfd, void *addr, bfd_size_type len, > +. int prot, int flags, file_ptr offset); The return type here should be void*, not plain void. > +static void * > +cache_bmmap (struct bfd *abfd ATTRIBUTE_UNUSED, > + void *addr ATTRIBUTE_UNUSED, > + bfd_size_type len ATTRIBUTE_UNUSED, > + int prot ATTRIBUTE_UNUSED, > + int flags ATTRIBUTE_UNUSED, > + file_ptr offset ATTRIBUTE_UNUSED) > +{ > + void *ret = (void *)-1; Gnu coding standards request a space between the cast and the -1. This also applies to several other casts throughout the patch. > + > + if ((abfd->flags & BFD_IN_MEMORY) != 0) > + abort (); > +#ifdef HAVE_MMAP > + else > + { > + FILE *f = bfd_cache_lookup (abfd, CACHE_NO_SEEK_ERROR); > + if (f == NULL) > + return ret; > + > + ret = mmap (addr, len, prot, flags, fileno (f), offset); > + if (ret == (void *)-1) > + bfd_set_error (bfd_error_system_call); > + } > +#endif > + > + return ret; > + Since abort() doesn't return, the else clause and extra level of nested braces is superfluous here. Straight-line code would be cleaner IMO. Anyway, that's minor stuff, as I said; the patch looks fundamentally sound to me, but I can't OK it for you. cheers, DaveK