[haiku-commits] Re: haiku: hrev43302 - src/kits/network/libnetapi headers/os/net

  • From: Alex Wilson <yourpalal2@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 21 Nov 2011 16:03:18 -0700

> +namespace BPrivate {
> +
> +
> +class SSL {
> +public:
> +       SSL()
> +       {
> +               SSL_library_init();
> +
> +               int64 seed = find_thread(NULL) ^ system_time();
> +               RAND_seed(&seed, sizeof(seed));
> +       }
> +};
> +
> +
> +static SSL sSSL;

I assume the goal of sSSL is to have the SSL lib automagically init
for clients? I don't think this is the best way to do it, since this
could potentially be a slow operation, and not everyone using
libnetapi will be using SSL. Furthermore, if someone writes code with
a static object that assumes ssl is initialized, then their code could
break, depending on the static initialization order. A lazy
initialized static class member is a better option, I think. This
allows for idempotent, only-on-demand loading, and solves the static
initialization order issue.

Having client code (SecureSocket, for instance) call a method like
BPrivate::SSL::Init() is also more self-documenting, removing a bit of
magic for others who might want to hack on this code.

> +BSecureSocket::BSecureSocket(const BSecureSocket& other)
> +       :
> +       BSocket(other)
> +{
> +       // TODO: this won't work this way!
> +       fPrivate = 
> (BSecureSocket::Private*)malloc(sizeof(BSecureSocket::Private));
> +       if (fPrivate != NULL)
> +               memcpy(fPrivate, other.fPrivate, 
> sizeof(BSecureSocket::Private));
> +       else
> +               fInitStatus = B_NO_MEMORY;
> +}

This isn't a great TODO, although I realize you're likely to be the
one that solves it, it doesn't always happen that way, so our TODO's
should be verbose enough that anyone coming across it will have a good
idea of what needs to be done.

I also wondered while looking at this code why you're not using
new(std::nothrow) with a copy constructor to make a copy of
other.fPrivate. Maybe using malloc instead of new relates to your
TODO, I'm not sure...

Anyway, this is a cool addition. Is the goal to one day apply this to
Web+ to help getting away from Curl, or is this targeted towards Mail?

--Alex

Other related posts: