[parted-devel] [PATCH] Properly sync partitions with operating system

[Joel Granados]

On Thu, Feb 19, 2009 at 02:20:05PM +0100, Jim Meyering wrote:
> Joel Granados Moreno <jgranado at redhat.com> writes:
> > diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> > index 4a8c9e7..35ef1a5 100644
> > --- a/libparted/arch/linux.c
> > +++ b/libparted/arch/linux.c
> > @@ -279,6 +279,9 @@ struct blkdev_ioctl_param {
> >                  || (M) == SCSI_CDROM_MAJOR                              \
> >                  || ((M) >= SCSI_DISK1_MAJOR && (M) <= SCSI_DISK7_MAJOR))
> >
> > +/* Maximum number of partitions supported by linux. */
> > +#define MAX_NUM_PARTS		16
> > +
> >  static char* _device_get_part_path (PedDevice* dev, int num);
> >  static int _partition_is_mounted_by_path (const char* path);
> >
> > @@ -2261,45 +2264,91 @@ _blkpg_remove_partition (PedDisk* disk, int n)
> >                                      BLKPG_DEL_PARTITION);
> >  }
> >
> > +/*
> > + * The number of partitions that a device can have depends on the kernel.
> > + * If we don't find this value in /sys/block/DEV/range, we will use our own
> > + * value.
> > + */
> > +static int
> > +_device_get_partition_range(PedDevice* dev)
> > +{
> > +	int		range;
> > +	char	path[512];
> > +	FILE*	fd;
> > +
> > +	if(snprintf(path, sizeof(path), "/sys/block/%s/range", dev->path) < 0)
> > +		return MAX_NUM_PARTS;
> > +
> > +	fd = fopen(path, "r");
> > +	if(!fd)
> > +		return MAX_NUM_PARTS;
> > +
> > +	if(fscanf(fd, "%d", range) == EOF)
> > +		return MAX_NUM_PARTS;
> > +
> > +	return range;
> 
> Oops.  you need a "&" before "range" above.
> Did this compile?  With gcc's -Wformat?

yep, thx.

> 
> Also, don't leak that stream+file descriptor.

right again, already fixed.

> E.g.,
> 
>     bool ok = fscanf (fp, "%d", &range) == 1;
>     fclose (fp);
>     return ok ? range : MAX_NUM_PARTS;
> 
> Also, while parsing via *scanf is not robust, if you do use it,
> the code must not accept random invalid input by setting "range" to 0.
> Hence the modified test "== 1" above.

Ok, will apply.
> 
> Also, "fd" is normally as an integer file descriptor variable.
> Please use something else, e.g., "fp" instead.

Ok
> 
> I'll look at the rest once you've adjusted.

thx for review.
-- 
Joel Andres Granados
Brno, Czech Republic, Red Hat.