Re: [PATCH]: Save boot record before writing to the dest_drive

[kashyap garimella]

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".

Thank you for the help. I followed your suggestion.  I thought of putting a check before freeing, but that did not remove the warning. 

ChangeLog entry:
 * util/i386/pc/grub-setup.c:  Added sections for saving the boot + embed sector before writing to them. Also added another 
section for restoring the boot+embed sectors, given the backup file. The backup file is relative to the root directory. 
 * grub-setup takes more options ( -Z to specify the backup file, -z to restore the backup file )

I did not use indent(1), because it indented many other parts of the code, which I did not write. I indented only my code.

Thank you,
Kashyap Garimella Jagannadh (irc: garimella )
Undergraduate student,
Department of Computer Science & Eng
Indian Instition of Technology (IIT), Madras.

patch_save_mbr_final
Description: Binary data