This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: How to get file descriptor from abfd?
- From: Dave Korn <dave dot korn dot cygwin at googlemail dot com>
- To: Paul Pluzhnikov <ppluzhnikov at google dot com>
- Cc: binutils at sourceware dot org, Tom Tromey <tromey at redhat dot com>, gdb-patches ml <gdb-patches at sourceware dot org>, Dave Korn <dave dot korn dot cygwin at googlemail dot com>
- Date: Wed, 10 Jun 2009 16:26:31 +0100
- 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>
Paul Pluzhnikov wrote:
> On Mon, Jun 1, 2009 at 10:54 PM, Paul Pluzhnikov<ppluzhnikov@google.com> 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