[parted-devel] Bug: Removal of BLKPG causes regression of ability to manipulate disks with other partitions in use

[Phillip Susi]

On 3/30/2010 3:58 AM, Jim Meyering wrote:
> I'll make the request once again:
> Please give details of precisely how you test this (I assume you do).
> Best is to give a script like one of those in tests/, but a
> sequence of parted commands would be fine, too.
> This will require root permissions and the use of the scsi_debug module,
> so you may find it easiest to base a test script on one of the
> existing 5 that do that:
>   tests/t3000-resize-fs.sh
>   tests/t3200-type-change.sh
>   tests/t9010-big-sector.sh
>   tests/t9020-alignment.sh
>   tests/t9030-align-check.sh

I have not looked at the automated testing at all.  I tested parted
interactively on a real disk by adding and removing partitions while
sometimes forcing one or more to be in use by spawning a dd process with
a command like:

dd of=fifo < /dev/sdd

Which caused the shell to open the drive then dd blocks trying to open a
named pipe I created with mkfifo.  I made sure to test removing the
first and last partition both unused and while the partition next to it
was in use.  I tested both msdos and gpt partition tables, and in the
case of msdos, made sure to test the case where removing an extended
partition would renumber the next one which was in use.  This last case
resulted in the warning and the open partition remained with the higher
number for the time being.  Subsequently closing the partition and
either having fdisk force a BLKRRPART or using parted to make further
changes with BLKPG successfully refreshed the kernel partition table.

Oh that reminds me, is there a way to force the partition number used
with mkpart instead of choosing the first number not in use already?  I
ended up accidentally doing some testing with higher numbered partitions
without the lower ones since I had already removed them and thought it
might be a good test case to be able to artificially create say,
partition 27 on a gpt disk by itself.

>> +                                /* get start and length of existing partition */
>> +                                dev_name = _device_get_part_path (disk->dev, i);
> 
> Don't segfault when dev_name is NULL.

How can it be NULL, and what would be the proper recovery?  Throw a bug
exception and return?

>> +                                fd = open (dev_name, O_RDONLY);
> 
> You must free dev_name here, to avoid a leak.

Oh, I didn't realize _device_get_part_path() duplicated the string, I
thought it just returned the existing copy which would be freed when the
device object was.