Re: [PATCH] Replace explicit version checks with feature checks

[Jonathan Bastien-Filiatrault]

Simon Josefsson wrote:
> > > Quick comments:
> > >
> > > * Please add a comment describing what dump_bytes does, and preferrably
> > >   make it use _gnutls_bin2hex for hex encoding.  Is there a fixed-size
> > >   limit of 128 in the function?  If so, it would be nice to fix that.
> > I was not aware of the existence of the _gnutls_bin2hex function. The
> > fixed buffers in dump_bytes are emptied at the end of each line and
> > their maximum size is bounded by the number of bytes per line (16), so
> > it could in theory print  256^sizeof(size_t) bytes. I will fix
> > gnutls_buffers.c to use _gnutls_bin2hex.
>
> Great.
>
> > > * What does the 'bufel' variable name refers to?  It looks non-english
> > >   to me, so I'd prefer using 'buf'.
> > A mbuffer is a chain of buffer elements. I use buf to refer to the whole
> > chain of buffers (mbuffer_head_st) and bufel to refer to a buffer
> > element (mbuffer_st). I don't mind changing the variable names if they
> > really bother you.
>
> No, that's fine.  I just didn't understand the "buffer element"
> shortening to bufel.  I suggest putting in a short comment at the top of
> gnutls_mbuffers.c about this in case others wonder.
>
> > > * Some comments at the top of gnutls_mbuffers.? what it does would be
> > >   good.
> > Sure, I can do that. I was planning on adding those comments later on
> > anyway.
>
> Please add them and post an updated patch.  I think it is close to being
> pushed.

Expect something soon on that end. I will notify you when I update my 
buffers branch with something viable.

> Btw, do you expect more changes to the buffer handling for DTLS?  I'm
> thinking that maybe we should release GnuTLS 2.10.0 once the TLS 1.2
> support is working, which may happen soon, and I'm not sure how many of
> these patches makes sense in a GnuTLS 2.10.0 that wouldn't support DTLS.
> Of course, if you can finish DTLS support, that would be great, but I
> think complete TLS 1.2 is important enough to warrant a new stable
> release in the near future.

Yes, there are more changes on the way, there is still the read() path 
that needs a whole lot of cleaning (with industrial grade bleach). The 
clean buffers interface is much nicer regardless of whether DTLS is 
there or not. The library user will of course see no change in the API 
with the brand new buffers. In other words the new buffers and DTLS are 
pretty much independent and can be merged in two steps with no problem 
at all.

> /Simon

Take care,
Jonathan