Re: [PREVIEW] Encapsulate SSL configuration settings

On 07/02/2013 06:49 AM, Amos Jeffries wrote:

> This creates an Ssl::Config class to manage SSL configuration settings
> and creates a ConfigOptions sub-class to hold the settings which are
> needed passing around in the SSL code.

The above description does not match the patch: We already have an
Ssl::Config class and the ConfigOptions you are adding is not a subclass
of that.

> +/// parse and store the configuration options used
> +/// for generating an SSL context
> +class ConfigOptions

Thank you for writing a class description!

I suggest renaming this class to Ssl::ContexConfig or
Ssl::ContextOptions to distinguish it from the existing Ssl::Config class.

However, if this class is actually not specific to SSL context (I have
not checked) but is specific to a "port" (as in http_port or
https_port), then something like Ssl::PortOptions would be better. We do
not have an outgoing "port" configuration option, but we kind of have
that concept so it may work for both outgoing and incoming connections.

I would also remove "parse and store the" from the description to avoid
limiting it too much and to focus on the class meaning rather than a few
of its actions. For example:

/// manages SSL context configuration options
class ContextConfig

> TODO:
> * implement dump/free functions properly
> * implement parse for backward-compatible loading of the old DIRECT SSL
> settings options.

* implement assignment operator for the new class

Thank you,

Alex.
Received on Tue Jul 02 2013 - 18:11:50 MDT