On Thu, Nov 20, 2008 at 14:17, Karel Zak <kzak@xxxxxxxxxx> wrote: > On Thu, Nov 13, 2008 at 08:01:34PM +0100, Kay Sievers wrote: >> 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? Sounds good, yes. > - 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? We can do that, sure. One thing to keep in mind for your new lib is to allocate all the results individually and return them all to the caller, I guess. So cleaning or copying results like here would not be needed, and the caller can decide what to do with conflicting results. > - .. or at least never use the second for() when the fist for() found > nothing ;-) Good point. :) > For example see the patch below (it's incomplete, volume_id_cpy() and > volume_id_remresult() are not implemented). > > --- 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); Wouldn't that copy, and later return the last result? Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html