GNOME Bugzilla – Bug 501853
g_checksum_get_digest docs
Last modified: 2007-12-20 15:15:58 UTC
The docs for g_checksum_get_digest don't mention that the data returned in the |digest| location needs to be g_free'd after use. And it also should note that one needs to initialise it to NULL before the function call. (Also IMHO a signature like guint8 *g_checksum_get_digest (GChecksum*, gsize *len) would have been more natural for this function.)
I also wonder what the motivation is for the descrepancy between get_digest (which returns allocated memory) and get_string (where the checksum retains ownership of the memory)
(In reply to comment #0) > The docs for g_checksum_get_digest don't mention that the data returned in the > |digest| location needs to be g_free'd after use. And it also should note that > one needs to initialise it to NULL before the function call. yeah, my fault. I'll update the documentation. > (Also IMHO a signature like > > guint8 *g_checksum_get_digest (GChecksum*, gsize *len) > > would have been more natural for this function.) I really wish you had commented on bug 443648 - now I have to dig out why I did that. :-) the get_digest() function original signature was: boolean get_digest (out byte digest, out size_t len) but back then the function had the ability to fail, so I guess it just remained like that even though I removed the boolean return value. as I said on bug 443648, I have no strong feelings on the get_digest() API so it can be changed to guint8* g_checksum_get_digest (GChecksum *checksum, gsize *digest_len); as far as I'm concerned. (In reply to comment #1) > I also wonder what the motivation is for the descrepancy between > get_digest (which returns allocated memory) and > get_string (where the checksum retains ownership of the memory) get_digest() copies the contents of the digest buffer, while get_string() just returns a pointer to the internal data. instead of copying using allocated memory I guess it could take a guint8** and memcpy() the buffer.
Created attachment 101254 [details] [review] proposed patch Add g_checksum_type_get_length(). Change g_checksum_get_digest to use a passed-in buffer instead of allocating memory.
the patch looks fine for me
I wonder if the g_assert_not_reached () is optimal in the switch statements, considering we may expand the enum later. How about we make get_length() return -1 in that case, and document that to mean "type not supported" ? Other than that, looks fine.
* glib/gchecksum.c: (g_checksum_type_get_length), (g_checksum_get_digest): * glib/gchecksum.h: * glib/glib.symbols: * tests/checksum-test.c: (test_checksum): Add g_checksum_type_get_length, and change g_checksum_get_digest to use a provided buffer instead of returning allocated memory. Bug #501853. Should g_checksum_new return NULL for unsupported types, instead of the current g_return_val_if_fail() ?
2007-12-20 Matthias Clasen <mclasen@redhat.com> * glib/gchecksum.[hc] (g_checksum_new): Return NULL when the checksum_type is unknown. (#501853)