Created attachment 336292 [details] [review]
gchecksum: Adds SHA-384 support

Do you have a use case where this is important to have ?

libdnf (former libhif) -> hence rpm-ostree and dnf should have support for sha384 repo checksums.

@@ -47,6 +48,7 @@ typedef enum {
   G_CHECKSUM_MD5,
   G_CHECKSUM_SHA1,
   G_CHECKSUM_SHA256,
+  G_CHECKSUM_SHA384,
   G_CHECKSUM_SHA512
 } GChecksumType;

This breaks the ABI since it changes the SHA512 enum value.

(In reply to Christian Persch from comment #4)
> @@ -47,6 +48,7 @@ typedef enum {
>    G_CHECKSUM_MD5,
>    G_CHECKSUM_SHA1,
>    G_CHECKSUM_SHA256,
> +  G_CHECKSUM_SHA384,
>    G_CHECKSUM_SHA512
>  } GChecksumType;
> 
> This breaks the ABI since it changes the SHA512 enum value.
Oh, I will reorder it.

(In reply to Igor Gnatenko from comment #3)
> libdnf (former libhif) -> hence rpm-ostree and dnf should have support for
> sha384 repo checksums.

Ok, if we have a user, seems fine to add

Created attachment 336984 [details] [review]
gchecksum: Add SHA-384 support

v2: Put G_CHECKSUM_SHA384 after G_CHECKSUM_SHA512 to not break ABI.

References: https://bugzilla.gnome.org/show_bug.cgi?id=771997
Signed-off-by: Igor Gnatenko <ignatenko@src.gnome.org>

Review of attachment 336984 [details] [review]:

ok. thanks for adding tests. Please hold off with committing until I have done a 2.50.1 and branched

I reuse Sha512sum* for SHA384, so probably should reuse sha512_sum_digest(), but then it would memcpy() 512 bits instead of 384, but then we could have less error-possibility in the code.

master is open now

Review of attachment 336984 [details] [review]:

The unit test fails:

/checksum/SHA384/0: OK
/checksum/SHA384/1: OK
/checksum/SHA384/2: 
(/home/seb/gnome/glib/glib/tests/.libs/lt-checksum:5552): GLib-CRITICAL **: g_checksum_update: assertion 'checksum != NULL' failed

Created attachment 345092 [details] [review]
gchecksum: Adds SHA-384 support

Signed-off-by: Igor Gnatenko <ignatenko@src.gnome.org>

Created attachment 345093 [details] [review]
gchecksum: add SHA-384 support

Signed-off-by: Igor Gnatenko <ignatenko@src.gnome.org>

(In reply to Sébastien Wilmet from comment #11)
> Review of attachment 336984 [details] [review] [review]:
> 
> The unit test fails:
> 
> /checksum/SHA384/0: OK
> /checksum/SHA384/1: OK
> /checksum/SHA384/2: 
> (/home/seb/gnome/glib/glib/tests/.libs/lt-checksum:5552): GLib-CRITICAL **:
> g_checksum_update: assertion 'checksum != NULL' failed
I can't reproduce this one... But anyway I rebased on top of master, double-checked it and ran tests and non fails.

Review of attachment 345093 [details] [review]:

Looks good to commit now that master is open. The tests pass.

Created attachment 345417 [details] [review]
ghmac: Add support for G_CHECKSUM_SHA384 to GHmac

It has the same block size as SHA-512, so it just needs a new case in
the switch, some documentation updates, and the test vectors from RFC
4868.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

We’d better add GHmac support for SHA-384 at the same time, otherwise calling g_hmac_new(G_CHECKSUM_SHA384) will cause an assertion failure. Simple patch attached.

Pushed in master:

* 77945667f ghmac: Add support for G_CHECKSUM_SHA384 to GHmac
* 1f9a9f8ce gchecksum: add SHA-384 support

@@ -47,6 +48,7 @@ typedef enum {
   G_CHECKSUM_MD5,
   G_CHECKSUM_SHA1,
   G_CHECKSUM_SHA256,
+  G_CHECKSUM_SHA384,
   G_CHECKSUM_SHA512
 } GChecksumType;

So commit 1f9a9f8ce56a7996eea67a2d79ce70f03ecc61ac broke the ABI, as I had already pointed out in comment 4.

And it uses Since: 2.51 whereas usually we use the stable version number, not the unstable one.

(In reply to Christian Persch from comment #20)
> And it uses Since: 2.51 whereas usually we use the stable version number,
> not the unstable one.
Oh god, why it made there? I've fixed it before.. =(

So sorry about this.

* 3e32350b5 gchecksum: don't break ABI by inserting new enum member in the middle