[Debburn-devel] strange memsets

[Thomas Schmitt]

Hi,

i am not involved in genisoimage, but in libjte which stems from
the JTE code in genisoimage.

So i comment on both code bases, refering to
  http://anonscm.debian.org/viewvc/debburn/cdrkit/trunk/genisoimage/
(last change: Thu Apr 22 23:32:47 2010 UTC)

--------------------------------------------------------------------

> +++ wrk/genisoimage/jte.c    2013-02-22 13:31:50.967667591 +0100
> @@ -280,7 +280,7 @@ extern int list_file_in_jigdo(char *file
> 
> -    memset(md5, 0, sizeof(md5));
> +    memset(md5, 0, sizeof(*md5));

Your change seems wrong, because the variable md5 is declared by

extern int list_file_in_jigdo(char *filename, off_t size, char **realname, unsigned char md5[16])

The statement shall zero all 16 bytes of md5, not only one byte as
would happen with  sizeof(*md5)  ( == sizeof(unsigned char)).

If the compiler really sees sizeof(md5) == 1, then one should
explicitely state 16 instead of sizeof(md5).


--------------------------------------------------------------------

> +++ wrk/genisoimage/md5.c    2013-02-22 13:15:29.979935133 +0100
> @@ -183,7 +183,7 @@ mk_MD5Final (unsigned char digest[16], s
> -    memset(ctx, 0, sizeof(ctx));    /* In case it's sensitive */
> +    memset(ctx, 0, sizeof(*ctx));    /* In case it's sensitive */

This change seems indeed valid, because we have as declaration

void
mk_MD5Final (unsigned char digest[16], struct mk_MD5Context *ctx)

The original statement would only zero 4 or 8 bytes.
Memory-wise harmless, because sizeof(struct mk_MD5Context) >= 70.
But it does not fulfill its job properly.

I will change this in my local copy of libjte which goes into
GNU xorriso.

Oh my ... where do i have the test code for Jigdo production ... ?


--------------------------------------------------------------------

> +++ wrk/libedc/edcspeed.c    2013-02-22 10:46:52.497686387 +0100
> @@ -35,7 +35,8 @@ static int encspeed()
> -    memset(sect, 0, sizeof(sect));
> +    //unneccessary, immediately rewritten again
> +    //memset(sect, 0, sizeof(sect));

Well, that's outside of JTE code. Not my sandbox.

But i would say this is not only unneccessary but also wrong.
One should probably remove that line without leaving a comment.
(Does genisoimage coding style allow C++ comments ?)

--------------------------------------------------------------------


Have a nice day :)

Thomas