[rfa] Add the bfd_iovec

[Andrew Cagney]

>   From my reading of your code, you seem to have removed the caching
> functionality from the bfd layer and moved it into a specialized iovec.

Only one IOVEC required the mechanism, and there, for a very specalized 
reason (as you note) - a shortage of file descriptors.  By moving that 
complexity into the FILE IOVEC, BFD proper was simplified.

When it turns out that a generalized FD-cache mechanism is needed then, 
the person needing the mechanism can add it :-) (at present we've only 
memory, FILE and GDB-inferior IOVECs and only FILE has the problems).

Anyway, and regardless, I think that such a mechanism belongs in the 
lower layers (IOVEC) and should not be directly tied to BFD.  By keeping 
it separate from BFD clients (i.e., GDB) use the underlying FD-cache to 
manage other open files - the source code.

> I
> don't think this is the right decision.  Since we're making an abstract I/O
> API that is modelled on the standard FILE * functions, and since the caching
> functionality that already exists works using FILE * functions, it would
> make more sense to me if the caching remained layered *over* the iovec
> abstraction.  That way caching could be applied to any iovec regardless of
> the underlying IO method, couldn't it?  The iovec should maybe contain a
> flag to indicate to the caching layer whether bfd-fd-caching is a win or not
> for the underlying IO method.


>   I also want to talk about the approach to iovecs with limited
> functionality - read-only, for instance, since you introduce one of those
> later.  ISTM that there are three sensible ways of handling this situation: 

ISTM?

> 1) You can specify that every iovec has to have the function pointers set
> for every callback, and supply dummy routines that return error indications
> for functionality such as bwrite () that doesn't exist;
> 
> 2) You can specify that if a function isn't provided the pointer in the
> iovec should be NULL, and include a guard test at every site that calls
> through the iovec table; or
> 
> 3) Same as 2), but you provide a set of wrappers, so instead of calling
> 
> +  nwrote = abfd->iovec->bwrite (abfd, ptr, size);
> 
> you'd say
> 
> +  nwrote = bfd_iovec_bwrite (abfd->iovec, abfd, ptr, size);
> 
> and supply a wrapper function that incorporates the error checking from 1)
> 
> +file_ptr
> +bfd_iovec_bwrite (struct iovec *vec, struct bfd *abfd,
> +	      const void *where, file_ptr nbytes )
> +{
> +  if (vec->bwrite)
> +    return vec->bwrite (abfd, where, nbytes);
> +  return -1;
> +}

> thereby consolidating all the error checking for all iovecs with
> unimplemented functions in one place.  I haven't verified if you'd ever need
> to pass in an iovec that wasn't the one from abfd, or if you could even
> simplify it to 
> 
> +  nwrote = bfd_iovec_bwrite (abfd, ptr, size);
> 
> +file_ptr
> +bfd_iovec_bwrite (struct bfd *abfd,
> +	      const void *where, file_ptr nbytes )
> +{
> +  if (abfd && abfd->vec && abfd->vec->bwrite)
> +    return abfd->vec->bwrite (abfd, where, nbytes);
> +  return -1;
> +}

As you note, each has advantages and disadvantages.  Where the interface 
external I'd have done what you suggest.  It isn't so I made it as 
simple and sparse as possible.

Also, one of the long ago original threads expressed concern at the call 
overhead (Ok I don't be believe it to be significant), but hey this 
looks leaner :-)

>   (BTW, since this is an internal function, it should maybe actually be
> called _bfd_iovec_bwrite instead.)
> 
>   I'm curious to know the reasoning behind your design decisions.  To me it
> seems that 3) is the cleanest and most generalized abstraction, and probably
> more future-proof / maintainable; the extra layer of wrappers may seem a bit
> of an overhead, but IMO it's pretty trivial compared to the actual io
> operation that's going to be taking place, and it provides a more robust
> interface for future iovecs that people may implement.
> 
>   Finally,
> 
> ------->snip!<------- 
> @@ -415,7 +593,12 @@
>    if (! BFD_SEND (abfd, _close_and_cleanup, (abfd)))
>      return FALSE;
>  
> -  ret = bfd_cache_close (abfd);
> +  /* FIXME: cagney/2004-02-15: Need to implement a BFD_IN_MEMORY io
> +     vector.  */
> +  if (!(abfd->flags & BFD_IN_MEMORY))
> +    ret = abfd->iovec->bclose (abfd);
> +  else
> +    ret = 0;
> ------->snip!<------- 
> 
>   Isn't it likely to break something if you check this in without doing so? 

I'm lost.  It's the memmory code, that's going to need many changes.

Andrew