Re: [PATCH] Removal of SquidMD5* code

On 18/03/2014 6:07 a.m., Kinkie wrote:
> Hi,
>
> helpers/basic_auth/NCSA/crypt_md5.cc
> there's an XXX hinting at some uncertainty. Is it stale or is there some doubts?

Still some doubts. I have not tested yet whether we can replace it.
Though I left it since its not directly MD5 code.

>
> helpers/basic_auth/RADIUS/basic_radius_auth.cc and elsewhere
> +#if HAVE_NETTLE_MD5_H
> +#include <nettle/md5.h>
> +#endif
> I know that the conventions mandate to #if-guard this include, but
> since it is mandatory and we do not provide an alternate
> implementation, does it make sense? It'll break regardless.

I wondered. But for now I am sticking with the guidelines. We have a
number of headers in this same situation and if we change the guidelines
again this can be done at that time.

>
> helpers/basic_auth/RADIUS/radius.h
> #define AUTH_VECTOR_LEN MD5_DIGEST_SIZE // 16
> I'd suggest to use a c-style comment for clarity.
>
>
> Food for thought: what about c++-wrapping the nettle API, at lieast
> for MD5 which we use extensively?

That would retain the SquidMD5& wrapper cross-dependency between src/
and helpers/ etc.
I am hoping to make the libcompat as small as possible. Even though it
means a few extra reinterpret_cast in code having to switch between
signed/unsigned char/int8_t types.

Amos
Received on Tue Mar 18 2014 - 03:07:28 MDT