[PATCH] sha2: new header <sha2.h>

[Mike Frysinger]

On 27 Mar 2015 14:39, Shawn Landden wrote:
> --- a/configure.ac
> +++ b/configure.ac
>
> -  nss_includes=-I$(nss-config --includedir 2>/dev/null)
> +  nss_includes=$(pkg-config --cflags-only-I nss)

you cannot hardcode `pkg-config`.  you must respect $PKG_CONFIG.  normally 
you'd use PKG_PROG_PKG_CONFIG, but that'd require an external pkg.m4 to be 
available, so do this instead earlier in the file:
	AC_PATH_TOOL([PKG_CONFIG], [pkg-config])

> --- /dev/null
> +++ b/crypt/align.h
>
> +#define put_be32(p, v)	do { *(uint32_t *)(p) = be32toh(v); } while (0)
> +#define put_be64(p, v)	do { *(uint64_t *)(p) = be64toh(v); } while (0)
> +#define put_be32(p, v)	do { \
> +	unsigned int __v = (v); \
> +	*((unsigned char *)(p) + 0) = __v >> 24; \
> +	*((unsigned char *)(p) + 1) = __v >> 16; \
> +	*((unsigned char *)(p) + 2) = __v >>  8; \
> +	*((unsigned char *)(p) + 3) = __v >>  0; } while (0)
> +#define put_be64(p, v)	do { \
> +	unsigned int __v = (v); \
> +	*((unsigned char *)(p) + 0) = __v >> 56; \
> +	*((unsigned char *)(p) + 1) = __v >> 48; \
> +	*((unsigned char *)(p) + 2) = __v >> 40; \
> +	*((unsigned char *)(p) + 3) = __v >> 32; \
> +	*((unsigned char *)(p) + 4) = __v >> 24; \
> +	*((unsigned char *)(p) + 5) = __v >> 16; \
> +	*((unsigned char *)(p) + 6) = __v >>  8; \
> +	*((unsigned char *)(p) + 7) = __v >>  0; } while (0)

these are like ... really bad.  they violate strict aliasing so hard.  just use 
the macros already available in endian.h.  like htobe32 and htobe64.  which you 
seem to use elsewhere in this patch already ;).

> --- /dev/null
> +++ b/crypt/sha2.h
>
> +typedef struct {
> +  char __internal_state[176];
> +} sha256_ctx __attribute__((aligned(16)));

this wasn't a problem before because you weren't installing the header, but now 
that you are, you have to use __aligned__.

applies below too.

> +/* sha256() writes 32 bytes to md and returns md.
> + * sha256_finish() writes 32 bytes to md and returns md.
> + * sha224_finish() writes 28 bytes to md and returns md.
> + * sha256_update() returns d.*/
> +void *sha256(const void *__restrict d, size_t n, void *__restrict md);
> +void sha256_init(sha256_ctx *s);
> +const void *sha256_update(sha256_ctx *__restrict s, const void *__restrict d, size_t n);
> +void *sha256_finish(sha256_ctx *__restrict s, void *__restrict md);
> +void *sha224_finish(sha256_ctx *__restrict s, void *__restrict md);

comments about the api should be split before each function rather than one big 
block.  you should also omit the names (like "d" or "n") as those cause 
namespace issues.

applies below too.

you should also update the manual to document these.

i did see some clean ups we might want to split out & merge regardless of 
exporting the sha2 API (like deleting the SWAP ugliness).  although, considering 
glibc already has this code in it, i don't have a problem with exporting them as 
proper symbols (assuming the API is reasonable).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20150328/5fc03109/attachment.sig>