[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Jonathan Bastien-Filiatrault
Subject: Re: [PATCH] Replace explicit version checks with feature checks
Date: Mon, 31 Aug 2009 14:51:44 -0400
User-agent: Mozilla-Thunderbird (X11/20090103)

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.


* 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
Sure, I can do that. I was planning on adding those comments later on

Please add them and post an updated patch.  I think it is close to being

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.


Take care,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]