Hi Mike,

Good work discovering this bug.  I've been working with GPT lately and had not been doing much testing with MSDOS partition tables, so this one slipped right by me.

I confirmed a crash problem when resizing a *logical* partition.

Resizing and *extended* partition worked successfully.

If this is what you find also, then we might wish to update the description to:

Parted crashing when opening Resize/Move dialog on logical partition
                                                   ^^^^^^^

Curtis

Summary updated as per Curtis' suggestion.

Created attachment 307773 [details] [review]
DEBUG: Assigning display_partitions_ref in activate_resize()

Hi Curtis,

Here's a debugging patch.  When I run this and open the Resize/Move
dialog I get this output:

D: Win_GParted::activate_resize()
D:  #1
D:  display_paritions[0].{device_path="/dev/sdc", partition_number=1, type=0(PRIMARY)}
D:  display_paritions[1].{device_path="/dev/sdc", partition_number=2, type=0(PRIMARY)}
D:  display_paritions[2].{device_path="/dev/sdc", partition_number=3, type=0(PRIMARY)}
D:  display_paritions[3].{device_path="/dev/sdc", partition_number=4, type=2(EXTENDED)}
D:  display_paritions[3].logicals[0].{device_path="/dev/sdc", partition_number=5, type=1(LOGICAL)}
D:  display_paritions[3].logicals[1].{device_path="/dev/sdc", partition_number=-1, type=3(UNALLOCATED)}
D:  display_paritions[4].{device_path="/dev/sdc", partition_number=-1, type=3(UNALLOCATED)}
D:  ext=3
D:  display_paritions[0].{device_path="/dev/sdc", partition_number=1, type=0(PRIMARY)}
D:  display_paritions[1].{device_path="/dev/sdc", partition_number=2, type=0(PRIMARY)}
D:  display_paritions[2].{device_path="/dev/sdc", partition_number=3, type=0(PRIMARY)}
D:  display_paritions[3].{device_path="/dev/sdc", partition_number=4, type=2(EXTENDED)}
D:  display_paritions[3].logicals[0].{device_path="/dev/sdc", partition_number=5, type=1(LOGICAL)}
D:  display_paritions[3].logicals[1].{device_path="/dev/sdc", partition_number=-1, type=3(UNALLOCATED)}
D:  display_paritions[4].{device_path="/dev/sdc", partition_number=-1, type=3(UNALLOCATED)}
D:  #2
D:  display_paritions[0].{device_path="/dev/sdc", partition_number=5, type=1(LOGICAL)}
D:  display_paritions[1].{device_path="/dev/sdc", partition_number=-1, type=3(UNALLOCATED)}
Segmentation fault (core dumped)


This looks like line 1759 where a reference is assigned to the logicals
partition vector is corrupting the parent display_partitions vector.

  1757                  std::cout << "D:  ext=" << ext << std::endl;
  1758                  print_display_partitions();
  1759                  display_partitions_ref = display_partitions[ext].logicals;
  1760          }
  1761          std::cout << "D:  #2" << std::endl;

Mike

Created attachment 307781 [details] [review]
Fix resize logical partition crash (v1)

Hi Mike,

From our discussions on IRC, we concluded that the line:

   display_partitions_ref = display_partitions[ext].logicals;

was actually changing both display_partitions_ref *AND* display_partitions.

The only way I could see how this would occur was if display_partitions_ref and display_partitions were one in the same pointer.

Upon examining the code further, it appears that this was indeed the case:

  std::vector<Partition> & display_partitions_ref = display_partitions;

The above line says to create a std::vector<Partition> named display_partitions_ref whose address is the same as display_partitions!

I think what was intended is the following line:

  std::vector<Partition> * display_partitions_ref = &display_partitions;

The above line says to create a std:vector<Partition> pointer and point it to the address of display_partitions.

The included patch makes this change, and has been tested to successfully resize primary, extended, and logical partitions on kubuntu 12.04.

Curtis

Created attachment 307859 [details] [review]
Fix resize logical partition crash (v2)

Hi Curtis,

You are indeed correct in you working out.  I was completely forgetting
how a reference actually worked, in particular these two points (stolen
from Wikipedia's Reference (C++) page):
* It is not possible to refer directly to a reference object after it is
  defined; any occurrence of its name refers directly to the object it
  references.
* Once a reference is created, it cannot be later made to reference
  another object; it cannot be reseated. This is often done with
  pointers.

If you don't mind I've changed the name of the variable to reflect that
it is a pointer now, rather than a reference.  Also extended the commit
message with a detailed explanation of what was wrong.  (To try to
remind me again what a reference actually is).

I've given the code a test and it worked as expected.  I'm ready to
commit this.

Thanks,
Mike

Hi Mike,

Thanks for testing the patch, adding more detail, and more clearly describing what is actually occurring.

I successfully tested resizing primary, extended and logical partitions using patch v2 from comment #5.  Please feel free to commit the patch.

I've made many mistakes with pointers and address references over the years, and I'm sure that I haven't made my last.  ;-)

Thanks again for catching this problem before our next official release.  Now all we need is a fully functioning SF.

Curtis

Patch v2 from comment #5 has been committed to the GParted GIT repo:

Adjust pointers to prevent crash when resizing a logical partition (#752587)
https://git.gnome.org/browse/gparted/commit/?id=c7c42f2cc5de27a673ddb31ae8f88b2d2ff97066

This enhancement was included in the GParted 0.23.0 release on August 3, 2015.