[PATCH] Replace explicit version checks with feature checks

[Simon Josefsson]

Jonathan Bastien-Filiatrault <joe at x2a.org> writes:

> Simon Josefsson wrote:
>> Jonathan Bastien-Filiatrault <joe at x2a.org> writes:
>> 
>>> Simon Josefsson wrote:
>>>> Jonathan Bastien-Filiatrault <joe at x2a.org> writes:
>>>>
>>>>> Simon Josefsson wrote:
>>>> How do I create a patch between your branch and master, to review it?  I
>>>> haven't worked with multiple repositories, but willing to learn.
>>>>
>>>> /Simon
>>> Alright, here is what you need to do:
>>>
>>>  - Use a somewhat-pristine git repository cloned from savannah or whatever.
>>>  - Add a remote: git remote add -f jonathan git://x2a.org/gnutls.git ;
>>> this adds an entry in the .git/config file and fetches refs and objects
>>> from my repository.
>>>  - Create a local branch tracking my remote repository: git branch
>>> --track buffers-redux jonathan/buffers-redux
>>>  - Now you can git checkout buffers-redux to switch to that branch or do
>>> a git diff master..buffers-redux . If you had not created a local branch
>>> you could instead do git diff master..jonathan/buffers-redux
>> 
>> Excellent, thank you!
>> 
>> 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.

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.

/Simon