Re: [PATCH 1/1] libfdisk: write mbr only when useful

[Karel Zak <kzak@xxxxxxxxxx>]

On Fri, Apr 08, 2022 at 02:10:04PM +0200, Philippe Reynes wrote:
>  static int gpt_write(struct fdisk_context *cxt, off_t offset, void *buf, size_t count)
>  {
>  	if (offset != lseek(cxt->dev_fd, offset, SEEK_SET))
> @@ -2079,6 +2092,8 @@ static int gpt_write_header(struct fdisk_context *cxt,
>  static int gpt_write_pmbr(struct fdisk_context *cxt)
>  {
>  	struct gpt_legacy_mbr *pmbr;
> +	struct gpt_legacy_mbr *current;
> +	int rc;
>  
>  	assert(cxt);
>  	assert(cxt->firstsector);
> @@ -2107,6 +2122,25 @@ static int gpt_write_pmbr(struct fdisk_context *cxt)
>  		pmbr->partition_record[0].size_in_lba =
>  			cpu_to_le32((uint32_t) (cxt->total_sectors - 1ULL));
>  
> +	current = malloc(sizeof(*current));
> +	if (!current)
> +		goto do_write;
> +
> +	rc = gpt_read(cxt, GPT_PMBR_LBA * cxt->sector_size,
> +		      current, cxt->sector_size);
> +	if (rc)

here is memory leak (for "current")

> +		goto do_write;
> +
> +	rc = memcmp(pmbr, current, sizeof(*current));

What about:

        rc = gpt_read(cxt, GPT_PMBR_LBA * cxt->sector_size, current, cxt->sector_size);

        if (rc == 0)
            rc = memcmp(pmbr, current, sizeof(*current));

        free(current);

        if (rc) {
           DBG(GPT, ul_debug("Same MBR on disk => don't write it"));
           return 0;
        }

    Karel

> +	free(current);
> +
> +	if (!rc) {
> +		DBG(GPT, ul_debug("Same MBR on disk => don't write it"));
> +		return 0;
> +	}
> +
> + do_write:
>  	/* pMBR covers the first sector (LBA) of the disk */
>  	return gpt_write(cxt, GPT_PMBR_LBA * cxt->sector_size,
>  			 pmbr, cxt->sector_size);
> -- 
> 2.25.1
> 

-- 
 Karel Zak  <kzak@xxxxxxxxxx>
 http://karelzak.blogspot.com