This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] Associate target_ops with target_fileio file descriptors
- From: Gary Benson <gbenson at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 24 Mar 2015 16:17:52 +0000
- Subject: Re: [PATCH v2] Associate target_ops with target_fileio file descriptors
- Authentication-results: sourceware.org; auth=none
- References: <20150318184538 dot GA32134 at blade dot nx> <1426867612-32278-1-git-send-email-gbenson at redhat dot com> <5511651F dot 4050400 at redhat dot com>
Pedro Alves wrote:
> On 03/20/2015 04:06 PM, Gary Benson wrote:
> > +
> > + /* Search for closed handles to reuse. */
> > + if (lowest_closed_fd >= 0)
> > + {
> > + for (index = lowest_closed_fd;
> > + VEC_iterate (fileio_fh_t, fileio_fhandles, index, fh);
> > + index++)
> > + if (is_closed_fileio_fh (fh->fd))
> > + break;
>
> You don't really need 'index'. If this walks using
> lowest_closed_fd instead, like:
>
> for (;
> VEC_iterate (fileio_fh_t, fileio_fhandles,
> lowest_closed_fd, fh);
> lowest_closed_fd++)
> if (is_closed_fileio_fh (fh->fd))
> break;
>
> Then the next time around we skip slots we already know are
> open.
>
> > + }
> > +
> > + /* Push a new handle if no closed handles were found. */
> > + if (index == total)
> > + {
> > + fh = VEC_safe_push (fileio_fh_t, fileio_fhandles, NULL);
> > + lowest_closed_fd = -1;
> > + }
> > +
> > + /* Fill in the handle and return its index. */
> > + fh->t = t;
> > + fh->fd = fd;
> > +
> > + return index;
> > +}
You do need another variable though. The function returns the
index of the file handle it just used, so you need to save that
across the bit that sets lowest_closed_fd to -1. I rearranged
things to this, but it's longer and I'd argue harder to follow:
/* Acquire a target fileio file descriptor. */
static int
acquire_fileio_fd (struct target_ops *t, int fd)
{
int total = VEC_length (fileio_fh_t, fileio_fhandles);
int index = total;
fileio_fh_t *fh, buf;
gdb_assert (!is_closed_fileio_fh (fd));
/* Search for closed handles to reuse. */
if (lowest_closed_fd >= 0)
{
for (;
VEC_iterate (fileio_fh_t, fileio_fhandles,
lowest_closed_fd, fh);
lowest_closed_fd++)
if (is_closed_fileio_fh (fh->fd))
break;
index = lowest_closed_fd;
}
/* Push a new handle if no closed handles were found. */
if (index == total)
{
fh = VEC_safe_push (fileio_fh_t, fileio_fhandles, NULL);
lowest_closed_fd = -1;
}
/* Fill in the handle and return its index. */
fh->t = t;
fh->fd = fd;
return index;
}
Do you prefer that?
I noticed you can do "lowest_closed_fd++" if you don't create a new
handle, so whichever way you like I'll add that micro-optimization.
Cheers,
Gary
--
http://gbenson.net/