[PATCH] Avoid use of uninitialized values in guid.c

Derek Atkins warlord at MIT.EDU
Sun Feb 13 18:13:33 EST 2005


I'm a bit concerned about a few aspects of this patch, so I haven't
applied it.  More comments inline..

Chris Shoemaker <c.shoemaker at cox.net> writes:

> +    char *tmp = "NULLGUID.EMPTY.";
>
> +    /* 16th space for '\O' */
>      for (i = 0; i < 16; i++)
> -      null_guid.data[i] = 0;
> +      null_guid.data[i] = tmp[i];

What's your reasoning for changing the NULL GUID like this?

> -    md5_process_bytes(de, sizeof(struct dirent), &guid_context);
> -    total += sizeof(struct dirent);
> +    md5_process_bytes(de->d_name, strlen(de->d_name), &guid_context);
> +    total += strlen(de->d_name);

I'm concerned here that you're reducing the amount of entropy
included.  Whereas before you were including the whole dirent object,
now you're just including the filename.  That's leaving out the inode
number, offset, and recordlen, which can all add entropy to the mix.

> -  guid_memchunk_init();
> +  /* Not needed: taken care of on first malloc.
> +  /* guid_memchunk_init(); */

The problem is that you don't know which one is called first,
guid_init() or guid_malloc().  If you want to protect the
memchunk_init() from being double-called, that's fine.

>  main (int argc, char **argv)
>  {
> -  scm_boot_guile(argc, argv, main_helper, NULL);
> +  /* scm_boot_guile(argc, argv, main_helper, NULL); */
> +  main_helper(NULL, argc, argv);
>    return 0;

Why did you remove the scheme loading here?

-derek
-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available


More information about the gnucash-patches mailing list