On Wed, Sep 23, 2009 at 1:38 PM, Mikko Rantalainen
<address@hidden> wrote:
I looked at the patch and it seems that the problem could be caused by
following code:
> + if (!backup_fp)
> + {
> + strcpy(grub_errmsg,"Unable to create the backup file");
> + goto unable_to_save;
> + }
> +
> + /* 8 bytes for embed_region.start, another 8 for core_size */
> + int backup_img_size = core_size + GRUB_DISK_SECTOR_SIZE + 8 + 8 + 1 /* seperation byte 0xff */ ;
> + backup_img = (char *) xmalloc (backup_img_size);
...
> +save_finish:
> + fclose (backup_fp);
> + free(backup_path);
> + free(backup_img);
Notice that you're using "goto" to skip initialization of backup_img and
you blindly free()ing it in "save_finish".
I think a nice way to fix this (if you want to use "goto") is to use
multiple clean up labels. Like this:
+save_cleanup_backup_img:
+ free(backup_img);
+save_cleanup_backup_fp:
+ fclose (backup_fp);
+save_cleanup_backup_path:
+ free(backup_path);
Note that you should release the resources in reverse order compared to
the order of acquiring the resources. If you need to bail out after you
have acquired the backup_fp but have not yet acquired the backup_img,
use "goto save_cleanup_backup_fp".