Bug 400330

Can you confirm whether that this happens before applying operations?

I can confirm that this happens by simply "export -> import" without applying operations. The steps I run:

1. start partitionmanager
2. select device
3. Device -> Export Partition Table: save file
4. Device -> Import Partition Table: load file
--> partition type of the partitions is loaded as "unknown"

Sometimes, only the second partition (ext4) is loaded as "unknown" but the first partition (fat32) is loaded correctly

I am actually surprised that sometimes partition is loaded correctly.

As far as I remember, there is no code to detect file system type in file.

We use "udevadm info --query=property /dev/deviceNode" to detect fs type but that only works with kernel devices, not files.

To me it looks like a race condition during import.

(In reply to Andrius Štikonas from comment #3)
> I am actually surprised that sometimes partition is loaded correctly.
> 
> As far as I remember, there is no code to detect file system type in file.
> 
> We use "udevadm info --query=property /dev/deviceNode" to detect fs type but
> that only works with kernel devices, not files.

Sorry, this comment was misleading, since the bug is about partition table import, not restoring partitions (which would be much harder to fix).

I launched my favorite debugger and found the problem:
mainwindow.cpp:1006: QLatin1String fsName = QLatin1String(rePartition.captured(4).toLatin1());

From the Qt documentation:
QLatin1String::QLatin1String(const QByteArray &str)
Constructs a QLatin1String object that stores str.
**The string data is not copied. The caller must be able to guarantee that str will not be deleted or modified as long as the QLatin1String object exists.**

So it is a mistake to construct a QLatin1String from a temporary.

As "fsType" is required to be a QString later anyway, a possible fix would be to simply create a QString:
QString fsName = rePartition.captured(4);

I tested this and it works.

But, this has explicitly changed in 4b4198c34a9f4880546242de330138cc11da8359 together with the creation of "fs*". The latter was reverted in 24237a6120e31f1051a07499f04c7391b877552c

(In reply to Christoph Vogtländer from comment #6)
> I launched my favorite debugger and found the problem:
> mainwindow.cpp:1006: QLatin1String fsName =
> QLatin1String(rePartition.captured(4).toLatin1());
> 
> From the Qt documentation:
> QLatin1String::QLatin1String(const QByteArray &str)
> Constructs a QLatin1String object that stores str.
> **The string data is not copied. The caller must be able to guarantee that
> str will not be deleted or modified as long as the QLatin1String object
> exists.**
> 
> So it is a mistake to construct a QLatin1String from a temporary.
> 
> As "fsType" is required to be a QString later anyway, a possible fix would
> be to simply create a QString:
> QString fsName = rePartition.captured(4);
> 
> I tested this and it works.
> 
> But, this has explicitly changed in 4b4198c34a9f4880546242de330138cc11da8359
> together with the creation of "fs*". The latter was reverted in
> 24237a6120e31f1051a07499f04c7391b877552c

Oh thanks for debugging this! I just started looking at this issue a few minutes ago. But you already finished investigation

Git commit 730b2a9992251cb2841cb8baa73d9dbaa8fbad54 by Andrius Štikonas.
Committed on 27/02/2019 at 23:15.
Pushed by stikonas into branch 'master'.

Fix race condition in reading fstype on partition table import.

Thanks to Christoph Vogtländer for reporting and fixing.

M  +1    -1    src/gui/mainwindow.cpp

https://invent.kde.org/kde/partitionmanager/commit/730b2a9992251cb2841cb8baa73d9dbaa8fbad54