apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe Jr." <wr...@rowe-clan.net>
Subject 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
Date Tue, 15 Dec 2009 01:19:32 GMT
Just a bit more detail on this one objection...

William A. Rowe Jr. wrote:
> 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 ...

And I think it's overkill.  If the 'incomplete' apr_crypto_t is defined to always
complete to a first member referencing the apr_crypto_driver_t, we just don't need
this uglyness.  A similar example was using pools as the first member of arbitrary
apr types to always ensure the pool-of could be extracted from an arbitrary type.

The implementation of apr_crypto_error can simply map apr_crypto_t to the predictable
initial elements, e.g. pool and driver.

Make sense?

Mime
View raw message