cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] request for review - CyaSSL patches for curl

From: Sajith T S <sajith_at_gmail.com>
Date: Tue, 31 Mar 2009 10:35:34 +0530

Daniel Stenberg <daniel_at_haxx.se> writes:

> Uh, where is these docs that explain this?

I'm looking for that myself. :)

> So what have you proven to work? You mention that you don't verify
> peers and some sites don't work. How much _does_ work?

Reason for this work was that I needed something to put inside a small
embedded box, something that can fetch files over https. This was a
proof of concept provisioning system. I had my own server running for
testing, that was found to be working. I tested against few random
https sites also, most of them appeared to work. The only exception so
far I know is https://login.yahoo.com.

Obviously I can't test this thing against the entire Internet! Guess a
test suite can help, but given the state of the patch, it's not going to
be anytime soon.

> I gave the cyassl code a quick glance just now. It seems it isn't
> thread-safe, do you know if that's so?

I don't have a conclusive answer to that rightaway, sorry.

> We'll of course credit all contributors. But what is perhaps more
> important is that there must be no actual ownership claims from those
> authors as then that piece of code would be GPL licensed (as that's
> what cyassl is licensed under) and I don't want GPL licensed code in
> libcurl.

Ok. While the library is obviously GPL licensed, I hope the examples
are not. Will get this clarified with the authors.

> Some nits that immediately struck me:
>
> A) why does the code set the socket to non-blocking when it already should
> always and unconditionally be non-blocking? (seems like a plain copy and
> paste from the cyassl example code that isn't needed)

True enough, that is copy and paste. Thought I'd clean everything up
and then post the patches, but like I'd been saying

> B) the function claims non-blocking connect but is in fact (stupidly) looping
> until it has connected, which is limited libcurl a bit since it actually
> supports for example OpenSSL doing non-blocking handshaking.

Uhh. Again, copy and paste...

> C) The configure.ac patch _removes_ some things that it shouldn't and
> instead introduces the old way of dealing with poll...

That could be because I ported the patch from 7.18.2's configure.ac.
What is that it removes it shouldn't, and what is the old way of dealing
with poll? I could try figuring that out myself, but getting some help
with this would be easier.

> Given my previous track record with yassl[*] (which is even claimed to
> be more complete) I'm not personally going to spend a lot of time on
> this work until someone tells me with some faith that this is actually
> a good and useful SSL library. But I'm willing to commit patches that
> look good.

My hope is that someone will find it useful and worthwhile to fix this
cruft. I will try to fix whatever I can within my limited skills, but
no one should hold their breath!

Thanks,
Sajith.

-- 
  9DB8FF06 : CB80 0BA6 7D13 B10A 6FBB  D43E B4D2 28AD 9DB8 FF06
Received on 2009-03-31