Phillip Susi wrote: > We were using the long depreciated HDIO_GETGEO ioctl on the > partition to get its start sector. Use the new BLKPG_GET_PARTITION > ioctl instead. This allows for disks > 2TB and partitioned loop > devices, which don't support HDIO_GETGEO. As a fallback when > BLKPG_GET_PARTITION fails or is not availible, try getting the > values from sysfs, and finally use BLKGETSIZE64 and HDIO_GETGEO > as a last resort. > > This modifies and superceeds Petr Uzel's patch: > libparted: fix reading partition start sector from kernel
Thanks. This looks good, too, but there were some syntactic problems: - added lines going beyond column 80 - compilation failed --enable-gcc-warnings due to shadowed "fd" decl I've fixed those with the following incremental: However, this is enough of an improvement that it deserves a NEWS entry. Would you care to write that? Also, since it is fixing a bug, I'll bet you can at least outline a test case that fails without your patch yet passes with it. Would you please do that, or better still, write a test suite addition? diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c index 1bd7274..39372bb 100644 --- a/libparted/arch/linux.c +++ b/libparted/arch/linux.c @@ -2454,7 +2454,8 @@ _sysfs_int_entry_from_dev(PedDevice const* dev, const char *entry, int *val) block device corresponding to PART and PART_BASE is the sysfs name of PART. Upon success, return true. Otherwise, return false. */ static bool -_sysfs_ull_entry_from_part(PedPartition const* part, const char *entry, unsigned long long *val) +_sysfs_ull_entry_from_part(PedPartition const* part, const char *entry, + unsigned long long *val) { char path[128]; char *part_name = linux_partition_get_path(part); @@ -2480,10 +2481,12 @@ _sysfs_ull_entry_from_part(PedPartition const* part, const char *entry, unsigned /* Get the starting sector and length of a partition PART within a block device - * Use blkpg if available, then check sysfs and then use HDIO_GETGEO and BLKGETSIZE64 - * ioctls as fallback. Upon success, return true. Otherwise, return false. */ + Use blkpg if available, then check sysfs and then use HDIO_GETGEO and + BLKGETSIZE64 ioctls as fallback. Upon success, return true. Otherwise, + return false. */ static bool -_kernel_get_partition_start_and_length(PedPartition const *part, unsigned long long *start, +_kernel_get_partition_start_and_length(PedPartition const *part, + unsigned long long *start, unsigned long long *length) { int fd = -1; @@ -2507,13 +2510,13 @@ _kernel_get_partition_start_and_length(PedPartition const *part, unsigned long l ok = _sysfs_ull_entry_from_part (part, "start", start); if (!ok) { struct hd_geometry geom; - int fd = open (dev_name, O_RDONLY); - if (fd != -1 && ioctl (fd, HDIO_GETGEO, &geom)) { + int dev_fd = open (dev_name, O_RDONLY); + if (dev_fd != -1 && ioctl (dev_fd, HDIO_GETGEO, &geom)) { *start = geom.start; ok = true; } else { - if (fd != -1) - close(fd); + if (dev_fd != -1) + close(dev_fd); free (dev_name); return false; } @@ -2631,7 +2634,8 @@ _disk_sync_part_table (PedDisk* disk) unsigned long long length; unsigned long long start; /* get start and length of existing partition */ - if (!_kernel_get_partition_start_and_length(part, &start, &length)) + if (!_kernel_get_partition_start_and_length(part, + &start, &length)) goto cleanup; if (start == part->geom.start && length == part->geom.length)