curl-library
RE: [PATCH v2] Add connection delay to Happy Eyeballs.
Date: Sun, 3 Nov 2013 19:32:17 +0000
On Sun, 03 Nov 2013, Björn Stenberg wrote:
> Actually, a separate fix to the "Whut?" bug was already posted
> four days ago in (http://curl.haxx.se/mail/lib-2013-10/0268.html,
> although for some reason the mail archive doesn't include the
> attached patch).
Sorry I missed your patch - At the time I could only find your larger patch
with this fix in.
A duplication of work and a bit of a wasted time on my part but, oh well!,
not to worry ;-)
> Also I respectfully disagree that replacing a switch with an
> if-else-if-else-if sequence is simplifying. Multiple nested if/elses are
> significantly more complex to read than a switch.
Normally I would agree with you, however:
a) The value being tested for in the if within the default clause is the
same value that is being tested in the switch / case clauses so I think it
is clearer for that part to be outside the switch or in an if else block
b) The values that are being compared are bitwise integers as opposed to
value integers which I also think is confusing in a switch block. This may
be why your original case clause didn't always work as expected - different
compilers will do different things with case value_a |value_b
c) The switch only has two case statements which makes it a little short for
a switch block in my opinion
d) At an assembler level I would expect the compiler to generate the same
code for if else if else if as your original switch statement did rather
than the extra compare (jne) as it would if the if is part of the default
clause
Please also note that what I generated is also in keeping with the curl
coding style - It's not necessarily how I would code professionally,
although off the top of my head I'm not sure what I would done in this
instance ;-)
In some good news, the 16:30 Windows auto build reduced its failed tests as
a result of the fix - it's back to what I think is normal for that build.
Kind Regards
Steve
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-11-03