After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 700228 - FAT16/32 labels are sometimes shown corrupted
FAT16/32 labels are sometimes shown corrupted
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
GIT HEAD
Other Linux
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2013-05-13 14:00 UTC by Mike Fleetwood
Modified: 2013-09-18 16:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix fat labels (v1) (20.74 KB, patch)
2013-05-16 11:41 UTC, Mike Fleetwood
none Details | Review

Description Mike Fleetwood 2013-05-13 14:00:35 UTC
GParted will sometimes show a corrupted FAT16/32 label and may also show
the following error on stdout because of a bug in mlabel:

    (gpartedbin:18116): glibmm-CRITICAL **: 
    unhandled exception (type Glib::Error) in signal handler:
    domain: g_convert_error
    code  : 1
    what  : Invalid byte sequence in conversion input


Effects:
    Fedora 14        mtools-4.0.13
    RHEL/CentOS 6.x  mtools-4.0.12
    Debian 6         mtools-4.0.12 (uninitialised memory contents is
                                    different so instead use label
                                    "1234567890" reproduce)
Does not effect:
    Fedora 18        mtools-4.0.17
    RHEL/CentOS 5.x  mtools-3.9.10

I think that it was fixed in mtools 4.0.14 as per this item in the NEWS
file:
    Fixed buffer size problem in mlabel


Steps to reproduce:

# mkdosfs -F16 /dev/sda7
mkdosfs 3.0.9 (31 Jan 2010)
# export MTOOLS_SKIP_CHECK=1
# mlabel ::123456 -i /dev/sda7
# mlabel -s :: -i /dev/sda7
 Volume label is 123456~1t


It is happening because mlabel is writing the label into an
uninitialised character array without a terminating NUL and then
writting this to disk.  On reading the label trailing junk is also
returned.


I'm working on a fix for GParted to workaround the bug in mtools.
Mike
Comment 1 Curtis Gedak 2013-05-13 15:15:03 UTC
Hi Mike,

At one point we investigated using dosfslabel, but there were problems with this tool as well.  See:
Bug #576616 - Cannot set label of fat32 partition with dosfstools

Curtis
Comment 2 Mike Fleetwood 2013-05-13 15:24:24 UTC
Hi Curtis,

I was just going to do a simple fix of padding the label with spaces
up to the maximum length as that is what the label on disk looks like
with later mtools.

I'll take a closer look at Bug #576616 and dosfslabel too, just in case.

Thanks,
Mike
Comment 3 Mike Fleetwood 2013-05-16 11:41:40 UTC
Created attachment 244394 [details] [review]
Fix fat labels (v1)

Hi Curtis,

Here's the patch set to fix this.  Patches are:
    Combine duplicate code for fat16/32
    Pad fat16/32 file system labels with spaces (#700228)
    Upper case fat16/32 file system labels

Tested of Fedora 14 and CentOS 5.9.  All fat16/32 operations are the
same after the merge.

Thanks,
Mike
Comment 4 Curtis Gedak 2013-05-16 16:29:14 UTC
Hi Mike,

While testing this patch I started to think about the third commit which forces the label to upper case.  This led me to test out my old copy of Windows NT 4.  It seems that Windows will permit both upper and lower case characters in a FAT16 label.

Based on this testing I do not think we should force the labels to upper case.  

What do you think?

Curtis
Comment 5 Curtis Gedak 2013-05-16 16:34:41 UTC
Hmm... it looks like Window NT 4 just places the label in "Sentence capitalization".  I will need to test a newer version of Windows....
Comment 6 Curtis Gedak 2013-05-16 16:46:41 UTC
Okay, it looks like Windows 7 displays a FAT32 label as upper case.  Windows NT 4 displayed the label in "Sentence capitalization".

For example:
  Fat16-Label
displays in Windows NT 4 as:
  Fat16-label
        ^

Mike,

If we are to force labels to upper case, should we be doing this in the GUI when the user enters the label?

I'm thinking of the principle of least surprise for the user.

Thanks,
Curtis
Comment 7 Mike Fleetwood 2013-05-17 07:43:53 UTC
Hi Curtis,

I agree with the principle of least surprise and that the right way is
for the text to appear in upper case as the user enters it when
required.  That requires more change than I wan't to do on this bug so
can you just drop the 3rd patch "Upper case fat16/32 ... labels" and
just apply the first 2 patches?

Thanks,
Mike
Comment 8 Curtis Gedak 2013-05-17 15:50:37 UTC
> <snip> can you just drop the 3rd patch "Upper case fat16/32 ... labels"
> and just apply the first 2 patches?

Hi Mike, you can consider this done.

I confirmed the original problem on Debian 6, and also confirmed that the first two commits in the patch from comment #3 address the problem.

The first two commits in the patch have been committed to the Gnome repository for inclusion in the next release of GParted.

The relevant git commits can be viewed at the following links:

Combine duplicate code for fat16/32
https://git.gnome.org/browse/gparted/commit/?id=519af1a7c08667c53e602012e2a49cbf5301f40c

Pad fat16/32 file system labels with spaces (#700228)
https://git.gnome.org/browse/gparted/commit/?id=7ede0ca3cc6fc705ad70868647ff7659255cfc12
Comment 9 Curtis Gedak 2013-05-17 16:06:35 UTC
As a result of combining the fat16 and fat32 code in commit 519af1a7c08667c53e602012e2a49cbf5301f40c, the fat32.cc file no longer exists, and hence is not available for translation to other languages.

This omission was spotted by running "make distcheck".

To address this omission, the following change has been committed to the Gnome repository for inclusion in the next release of GParted.

Remove fat32.cc from POTFILES.in (#700228)
https://git.gnome.org/browse/gparted/commit/?id=d9a3f879f915422df39923235aa6b09a8de6abd5
Comment 10 Curtis Gedak 2013-09-18 16:49:14 UTC
The patches to address this report have been included in GParted 0.16.2 released on September 18, 2013.