Bug 10362

Summary:	misdetects vfat volume label
Product:	hal	Reporter:	Martin Pitt <martin.pitt>
Component:	hald	Assignee:	Danny Kukawka <danny.kukawka>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium	CC:	danny.kukawka
Version:	unspecified
Hardware:	Other
OS:	Linux (All)
URL:	https://launchpad.net/ubuntu/+source/hal/+bug/69914
Whiteboard:
i915 platform:		i915 features:
Attachments:	patch to check if the label is empty ubuntu patch final patch

Description Martin Pitt 2007-03-21 03:45:39 UTC

In the original bug quoted above, the reporter has a vfat partition which never had a label, but hal detects it as

  info.product = 'Tp3WN_SQL' (string)
  volume.label = 'Tp3WN_SQL' (string)

blkid gets it correctly:

  /dev/evms/hdb5: UUID="7251-A453" TYPE="vfat"

Please see http://librarian.launchpad.net/5775694/hal.txt for the complete lshal output.

Comment 1 Danny Kukawka 2007-03-22 08:48:38 UTC

@Martin: I attach a patch, could you check with the user if this help?

Comment 2 Danny Kukawka 2007-03-22 08:49:34 UTC

Created attachment 9259 [details] [review]
patch to check if the label is empty

please check if this help

Comment 3 Martin Pitt 2007-03-23 02:19:47 UTC

The patch looks sane and harmless, so I'll apply it to the next Ubuntu upload for testing. However, AFAICS this would only actually make a difference if strdup_valid_utf8() would return garbage for an empty string? It's not immediately clear to me that this is the case after just a quick look at its implementation, but if it is, maybe it should be fixed in strdup_valid_utf8() itself?

Thank you! I'll report back.

Comment 4 Martin Pitt 2007-03-30 08:05:31 UTC

Created attachment 9383 [details] [review]
ubuntu patch

I modified that patch slightly, since I believe the current 'else' logic is incorrect due to the two nested if clauses. I added a 'return' right after the g_free(volume_label), and moved the contents of the else clause to the function top level.

Since I applied that patch on top of the one proposed in bug 10429, it now looks like this.

I didn't notice any apparent regressions, however, I cannot reproduce the original problem myself. I will ask the bug submitter for testing a current live CD, but that might take a while.

Comment 5 Danny Kukawka 2007-04-04 08:15:36 UTC

what print /lib/udev/vol_id --export /dev/evms/hdb5 ?

Comment 6 Martin Pitt 2007-04-17 05:09:21 UTC

Just for the record:

root@gremlin:/home/vyruss# /lib/udev/vol_id --export /dev/evms/hdb5
ID_FS_USAGE=filesystem
ID_FS_TYPE=vfat
ID_FS_VERSION=FAT32
ID_FS_UUID=7251-A453
ID_FS_LABEL=
ID_FS_LABEL_SAFE=

Comment 7 Martin Pitt 2007-04-17 05:10:13 UTC

Original Ubuntu reporter confirms that https://bugs.freedesktop.org/attachment.cgi?id=9383 fixed the issue.

Comment 8 Danny Kukawka 2007-04-17 06:27:25 UTC

Created attachment 9634 [details] [review]
final patch

I would commit this new version of the patch. This clean up the code and prevent double the same code. Should be the same as in patch from ubuntu, but with less code. 

Send this patch to the ML

Comment 9 Danny Kukawka 2007-04-18 12:27:04 UTC

commited to git HEAD and 0.5.9 branch

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.