[PATCH v3] bfd: Close the file descriptor if there is no archive fd

[H.J. Lu]

On Tue, Jul 27, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jul 26, 2021 at 6:44 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Mon, Jul 26, 2021 at 06:21:31AM -0700, H.J. Lu via Binutils wrote:
> > > Close the thin archive file descriptor since it can't be used directly
> > > to access archive members.  This avoids running out of file descriptors
> > > on thin archives with many archive members.
> > >
> > >       PR ld/28138
> > >       * plugin.c (bfd_plugin_close_file_descriptor): Close the thin
> > >       archive file descriptor.
> >
> > > +      /* Close the thin archive file descriptor since it can't be used
> > > +         directly to access archive members.  */
> >
> > At first reading, I understood the code better without the comment.
> > I think it would be better if you omit this comment and instead put a
> > comment before the while loop, if you feel you need a comment.
>
> I changed it to
>
>       /* Close the file descriptor if there is no archive plugin file
>          descriptor.  */
>       if (abfd->archive_plugin_fd == -1)
>         {
>           close (fd);
>           return;
>         }
>
> > Also, please don't pass in a NULL abfd for non-archives.  Instead put
> > the my_archive test in bfd_plugin_close_file_descriptor.
> >
>
> I prefer to keep it ASIS for
>
> ld/plugin.c:      bfd_plugin_close_file_descriptor (input->ibfd, input->fd);
>
>       ^^^^^^^^^^^ This is NULL.
>
> > I suspect you also need to tweak bfd_plugin_open_input, specifically
> >   /* Ruse the archive plugin file descriptor.  */
> >   if (iobfd != ibfd)
> > might be wrong for nested archives.
>
> It is handled correctly:
>
>  /* Reuse the archive plugin file descriptor.  */
>   if (iobfd != ibfd)
>     fd = iobfd->archive_plugin_fd;
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This isn't -1 only if there is a normal archive.
>   else
>     fd = -1;
>
>   if (fd < 0)
>
> > --
> > Alan Modra
> > Australia Development Lab, IBM
>
> Here is the v2 patch.  OK for master?

Here is the v3 patch with a testcase.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v3-0001-bfd-Close-the-file-descriptor-if-there-is-no-arch.patch
Type: application/x-patch
Size: 5877 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20210727/00b3e4f1/attachment-0001.bin>