On Fri, May 17, 2013 at 4:21 AM, David Aguilar <dav...@gmail.com> wrote:
> On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine <sunsh...@sunshineco.com> 
> wrote:
>> On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen <tbo...@web.de> wrote:
>>> On 2013-05-15 09.11, David Aguilar wrote:
>>>> +     ifndef NO_APPLE_COMMON_CRYPTO
>>>> +             APPLE_COMMON_CRYPTO = YesPlease
>>>> +     endif
>>>>       NO_REGEX = YesPlease
>>>>       PTHREAD_LIBS =
>>>>  endif
>>>> @@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
>>>>       LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
>>>>       LIB_H += ppc/sha1.h
>>>>  else
>>>> +ifdef APPLE_COMMON_CRYPTO
>>>> +     BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
>>>> +     SHA1_HEADER = <CommonCrypto/CommonDigest.h>
>>>
>>> Would it make sense to replace APPLE_COMMON_CRYPTO
>>> with COMMON_DIGEST_FOR_OPENSSL ?
>>>
>>> In the spirit of other Makefile-defines becoming Compiler defines,
>>> a random picked example:
>>> ifdef NO_STRTOULL
>>>         COMPAT_CFLAGS += -DNO_STRTOULL
>>> endif
>>
>> Not necessarily. Unlike NO_STRTOULL and cousins,
>> COMMON_DIGEST_FOR_OPENSSL is not a Git build tweak; it is merely a
>> (public) implementation detail of the Apple header [1] to magically
>> associate OpenSSL digest functions with CommonCrypto counterparts.
>> It's not the only such macro recognized by the Apple headers. For
>> instance, COMMON_DIGEST_FOR_RFC_1321 magically associates legacy MD5
>> digest functions with CommonCrypto counterparts.
>>
>> Further, as Junio noted elsewhere, David is using CommonCrypto for
>> HMAC replacements, not just for digest replacements, so a Makefile
>> knob with DIGEST in its name is not really appropriate. More
>> generally, David would like to find CommonCrypto replacements for all
>> the OpenSSL functionality, so a Makefile knob named after DIGEST is
>> too specific.
>>
>> These considerations motivated the original suggestion for a single
>> Git Makefile knob to enable/disable, as a unit, all CommonCrypto
>> replacements. Such a knob would naturally have COMMON_CRYPTO as part
>> of its name.
>
> This is a nice justification for taking v5 of this series over v6.

You will consider this bike-shedding (I don't), but the above also is
good justification for revising your HMAC patch to _not_ rely on
COMMON_DIGEST_FOR_OPENSSL, which is an implementation detail of your
SHA patch, rather than a proper build knob.

Similar to NO_STRTOULL and cousins, you should have a #define (such as
NO_APPLE_COMMON_CRYPTO or NO_COMMON_CRYPTO) which is consulted by your
HMAC patch and any future patches you submit to map CommonCrypto
counterparts to OpenSSL functions. The fact that you also must #define
COMMON_DIGEST_FOR_OPENSSL for the SHA patch is just an implementation
detail of that one patch; it is not relevant to the other patches.

-- ES
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to