[lwip-devel] [patch #8882] Vector improvements

[Joel Cunningham]

Follow-up Comment #42, patch #8882 (project lwip):

> Having a quick look, the patch seems good. The only thing I'm not certain of
is if it's OK to cast 'struct iovec' to the new 'struct netvector'. Especially
the "msg.w.vector++" code in lwip_netconn_do_writemore() could go wrong if the
(externally defined) struct iovec contained more fields than struct netvector
(or was differently aligned).

Good catch on the external definition.  I agree there could be problems.  I
don't have a good feeling for whether external struct iovec's are being used
currently.  The definition I had provided has a rudimentary check which would
only catch external definitions provided by a macro:


#if !defined(iovec)
struct iovec {
  void  *iov_base;
  size_t iov_len;
};
#endif


I see a couple of ways to solve this:

1) Add an additional define if the internal definition is being used.  If not
defined, we can copy the pointers/lengths from struct iovec into an equivalent
sized struct netvector in the sockets layer before calling netconn.  There
will be a performance penalty, but it would ensure struct iovec is only in the
sockets layer.  If internal definition is used, we can continue to use the
cast.  I can also add some sizeof compiler checks to ensure the structures
don't have size-mismatch in the internal case
2) Remove struct netvector and use struct iovec in netconn layer.  I'm
guessing this option is out given the previous discussion we had where I had
suggested moving it to def.h.  But this issue is evidence that keeping struct
iovec isolated in the sockets layer does have cost
3) Remove support for external definitions of struct iovec.  Not sure if this
is realistic, but the current method isn't that portable anyways.  We probably
need something like struct timeval has (#define LWIP_TIMEVAL_PRIVATE)

> I guess for clarity, we might want to omit the 'len' parameter for the
SENDPLUS/SENDMINUS calls...

I'll just change the calls in do_writemore to use 0 if there is no better
guidance.  Maybe the parameter can be removed in a subsequent cleanup

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?8882>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/