You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@apr.apache.org by "William A. Rowe Jr." <wr...@rowe-clan.net> on 2009/12/15 02:06:21 UTC

Re: svn commit: r890580 - in /apr/apr-util/branches/1.4.x: crypto/apr_crypto.c crypto/apr_crypto_nss.c crypto/apr_crypto_openssl.c include/apr_crypto.h include/private/apr_crypto_internal.h test/testcrypto.c

minfrin@apache.org wrote:
> Author: minfrin
> Date: Tue Dec 15 00:29:42 2009
> New Revision: 890580
> 
> URL: http://svn.apache.org/viewvc?rev=890580&view=rev
> Log:
> Backport r890579:
> Refactor the apr_crypto.h interface so that the apr_crypto_t structure
> remains private. Correctly reference the apr_crypto_t context as a context
> and not a factory.

Just a few API/ABI version compatibility problems;

> Modified: apr/apr-util/branches/1.4.x/include/apr_crypto.h
> URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.4.x/include/apr_crypto.h?rev=890580&r1=890579&r2=890580&view=diff
> ==============================================================================
> --- apr/apr-util/branches/1.4.x/include/apr_crypto.h (original)
> +++ apr/apr-util/branches/1.4.x/include/apr_crypto.h Tue Dec 15 00:29:42 2009
> @@ -222,15 +213,14 @@
>  APU_DECLARE(const char *)apr_crypto_driver_name (const apr_crypto_driver_t *driver);
>  
>  /**
> - * @brief Get the result of the last operation on a factory. If the result
> + * @brief Get the result of the last operation on a context. If the result
>   *        is NULL, the operation was successful.
> - * @param driver - driver to use
> - * @param factory - factory pointer will be written here
> + * @param f - context pointer
>   * @param result - the result structure
>   * @return APR_SUCCESS for success
>   */
> -APU_DECLARE(apr_status_t) apr_crypto_error(const apr_crypto_t *f,
> -        const apu_err_t **result);
> +APR_DECLARE(apr_status_t) apr_crypto_error(const apr_crypto_driver_t *driver,
> +        const apr_crypto_t *f, const apu_err_t **result);

This is ABI breakage from 1.4.0-dev to 1.4.1, please revert; extend with an
apr_crypto_error_ex for querying from an arbitrary interface.  It doesn't make
sense why we would want this, but ...

> @@ -240,11 +230,11 @@
>   * @param driver - driver to use
>   * @param pool - process pool
>   * @param params - array of key parameters
> - * @param factory - factory pointer will be written here
> + * @param f - context pointer will be written here
>   * @return APR_ENOENGINE when the engine specified does not exist. APR_EINITENGINE
>   * if the engine cannot be initialised.
>   */
> -APU_DECLARE(apr_status_t) apr_crypto_factory(const apr_crypto_driver_t *driver,
> +APU_DECLARE(apr_status_t) apr_crypto_make(const apr_crypto_driver_t *driver,
>          apr_pool_t *pool, const apr_array_header_t *params, apr_crypto_t **f);

ABI breakage, please add back the apr_crypto_factory stub fn to apr_crypto_make with
an @bug notice that it's deprecated.


> Modified: apr/apr-util/branches/1.4.x/include/private/apr_crypto_internal.h
> URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.4.x/include/private/apr_crypto_internal.h?rev=890580&r1=890579&r2=890580&view=diff
> ==============================================================================
> --- apr/apr-util/branches/1.4.x/include/private/apr_crypto_internal.h (original)
> +++ apr/apr-util/branches/1.4.x/include/private/apr_crypto_internal.h Tue Dec 15 00:29:42 2009
> @@ -41,6 +41,14 @@
>      apr_status_t (*init)(apr_pool_t *pool, const apr_array_header_t *params, int *rc);
>  
>      /**
> +     * @brief: fetch the most recent error from this driver.
> +     * @param f - context pointer
> +     * @param result - the result structure
> +     * @return APR_SUCCESS for success.
> +     */
> +    apr_status_t (*error)(const apr_crypto_t *f, const apu_err_t **result);

We don't expect this extended yet, but from a compatibility perspective please
relocate this new addition to the end of the array.  This way a 1.4.1 module
will load in 1.4.0-dev as deployed by users.  A module built for 1.4.0-dev can
reasonably not be expected to be released by anyone, so no harm from the missing
error element in other builds.

Re: svn commit: r890580 - in /apr/apr-util/branches/1.4.x: crypto/apr_crypto.c crypto/apr_crypto_nss.c crypto/apr_crypto_openssl.c include/apr_crypto.h include/private/apr_crypto_internal.h test/testcrypto.c

Posted by Graham Leggett <mi...@sharp.fm>.

On 30 Nov 2010, at 4:42 PM, Jim Jagielski wrote:

> As part of my push to get httpd 2.4 out the door, this is a
> major factor, so I plan on completing my review of the crypto
> API RSN.

Thanks for this.

The code has both a test suite, and a first implementation  
(mod_session_crypto.c).

A key part of the API, which has been misunderstood in the past, is  
the desire to be able to pre-initialise something (eg within httpd  
startup), and then use that pre-initialised state later (eg at httpd  
request time). The need for this may have not been clear when just  
looking at the API.

Regards,
Graham
--