On Fri, May 17, 2013 at 4:21 AM, David Aguilar <davvid@xxxxxxxxx> wrote: > On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen <tboegi@xxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html