Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re:

[Karel Zak <kzak@redhat.com>]

On Thu, Nov 13, 2008 at 08:01:34PM +0100, Kay Sievers wrote:
> I've committed  these changes to volume_id, and FAT32 accepts
> completely empty fsinfo signatures now.

 Thanks.

> We always probe for all filesystem types on volumes larger than a
> floppy disk. If we find multiple signatures and one of the detected
> filesystem types specifies that it can no co-exist with another known
> filesystem, like swap and FAT, we do not return a probing result.

 I see the patch (volume_id_probe_filesystem()) and a few things come
 to mind:

  - shouldn't be the relevant parts (label, uuid, version, ...) of
    the "struct volume_id" zeroized when you found a signature and
    before you call the next probing function?

  - it seems as overkill to use two for()s and probe two times for all
    filesystems. What about to store the first result and re-use it?

  - .. or at least never use the second for() when the fist for() found
    nothing ;-)

 For example see the patch below (it's incomplete, volume_id_cpy() and
 volume_id_remresult() are not implemented).

    Karel


diff --git a/extras/volume_id/lib/volume_id.c b/extras/volume_id/lib/volume_id.c
index aaa15e1..0f5cf2d 100644
--- a/extras/volume_id/lib/volume_id.c
+++ b/extras/volume_id/lib/volume_id.c
@@ -421,9 +421,12 @@ int volume_id_probe_filesystem(struct volume_id *id, uint64_t off, uint64_t size
 	 * smaller than a usual floppy disk.
 	 */
 	if (size > 1440 * 1024) {
+		struct volume_id first;
 		int found = 0;
 		int force_unique_result = 0;
 
+		memset(&first, 0, sizeof(first));
+
 		for (i = 0; i < ARRAY_SIZE(prober_filesystem); i++) {
 			int match;
 
@@ -437,20 +440,26 @@ int volume_id_probe_filesystem(struct volume_id *id, uint64_t off, uint64_t size
 					return -1;
 				}
 				found = 1;
+				volume_id_cpy(&first, id);
+				volume_id_remresult(id);
 			}
 		}
+		if (found) {
+			volume_id_cpy(id, &first);
+			goto found;
+		}
+		return -1;
 	}
 
 	/* return the first match */
 	for (i = 0; i < ARRAY_SIZE(prober_filesystem); i++) {
-		if (prober_filesystem[i].prober(id, off, size) = 0) {
-			info("signature '%s' detected\n", id->type);
+		if (prober_filesystem[i].prober(id, off, size) = 0)
 			goto found;
-		}
 	}
 	return -1;
 found:
 	/* If recognized, we free the allocated buffers */
+	info("signature '%s' detected\n", id->type);
 	volume_id_free_buffer(id);
 	return 0;
 }

-- 
 Karel Zak  <kzak@redhat.com>