README.md

@@ -116,8 +116,8 @@ The main types are:
 - `Image` and `Video` embed `Object`. The file extension decides which one a
   file becomes.
 
-A mounted filesystem is a DCF store when its root holds a `DCIM` or `M4ROOT`
+A mounted filesystem is a DCF store when its root holds a `DCIM` or
-directory. `dcf` never mounts a filesystem.
+`PRIVATE/M4ROOT` directory. `dcf` never mounts a filesystem.
 
 ## TODO
 
@@ -126,13 +126,9 @@ directory. `dcf` never mounts a filesystem.
   tar of a memory card that uses them, and I'll see what I can do.
 - Parse the DCF directory and file names, rather than listing every media file
   on the filesystem.
-- Look for `PRIVATE/M4ROOT` rather than `M4ROOT` when detecting a DCF store.
-- Return exactly the number of stores asked for. `GetDCFStores` returns one too
-  few when asked for two or more.
 - Read stores through `fs.FS`, with a `context.Context` to cancel a walk.
 - Return a plain slice from `GetDCFStores`, not a pointer to one.
-- Test the walk of a store and the detection of mount points. They only run
+- Test the walk of a store. It only runs against real mounted media today.
-  against real mounted media today.
 - Copy files off a card, and verify each copy against its SHA256 hash.
 
 ## License

pkg/dcf/helpers.go

@@ -28,45 +28,51 @@ func findAllMountPoints() ([]string, error) {
 }
 
 // findDCFMountPoints returns a list of strings of the paths of mountpoints
-// that contain a DCIM or M4ROOT directory, which is how we detect DCF stores.
+// that hold a DCF store. Its only argument is an integer of how many mountpoints
-// Its only argument is an integer of how many mountpoints to return. If the
+// to return. If the argument is 0, all mountpoints are returned.
-// argument is 0, all mountpoints are returned.
 func findDCFMountPoints(requestedCount int) ([]string, error) {
-	filteredMountpoints := []string{}
-
 	mountpoints, err := findAllMountPoints()
 	if err != nil {
 		return nil, err
 	}
 
+	return filterDCFMountPoints(mountpoints, requestedCount), nil
+}
+
+// filterDCFMountPoints returns the first requestedCount of the mountpoints that
+// hold a DCF store. If requestedCount is 0, it returns all of them.
+func filterDCFMountPoints(mountpoints []string, requestedCount int) []string {
+	filteredMountpoints := []string{}
+
 	for _, mountpoint := range mountpoints {
-		dcimPath := filepath.Join(mountpoint, "DCIM")
+		if !isDCFMountPoint(mountpoint) {
-		m4rootPath := filepath.Join(mountpoint, "M4ROOT")
+			continue
-		shouldKeep := false
-
-		_, err = os.Stat(dcimPath)
-		if err == nil {
-			shouldKeep = true
-
-			slog.Debug(fmt.Sprintf("Found DCIM directory at %s\n", dcimPath))
 		}
 
-		_, err = os.Stat(m4rootPath)
-		if err == nil {
-			shouldKeep = true
-
-			slog.Debug(fmt.Sprintf("Found M4ROOT directory at %s\n", m4rootPath))
-		}
-
-		if shouldKeep {
 		slog.Debug(fmt.Sprintf("Keeping mountpoint %s\n", mountpoint))
 
 		filteredMountpoints = append(filteredMountpoints, mountpoint)
-			if (requestedCount > 0) && (len(filteredMountpoints)+1 >= requestedCount) {
+		if (requestedCount > 0) && (len(filteredMountpoints) >= requestedCount) {
 			break
 		}
 	}
+
+	return filteredMountpoints
 }
 
-	return filteredMountpoints, nil
+// isDCFMountPoint reports whether the root of mountpoint holds a DCIM or
+// PRIVATE/M4ROOT directory, which is how we detect DCF stores.
+func isDCFMountPoint(mountpoint string) bool {
+	for _, directory := range []string{"DCIM", "PRIVATE/M4ROOT"} {
+		path := filepath.Join(mountpoint, directory)
+
+		info, err := os.Stat(path)
+		if err == nil && info.IsDir() {
+			slog.Debug(fmt.Sprintf("Found %s directory at %s\n", directory, path))
+
+			return true
+		}
+	}
+
+	return false
 }

pkg/dcf/helpers_internal_test.go

@@ -0,0 +1,85 @@
+package dcf
+
+import (
+	"os"
+	"path/filepath"
+	"slices"
+	"testing"
+)
+
+// newMountPoint returns a temporary directory holding the given directories.
+func newMountPoint(t *testing.T, directories ...string) string {
+	t.Helper()
+
+	root := t.TempDir()
+
+	for _, directory := range directories {
+		err := os.MkdirAll(filepath.Join(root, directory), 0o750)
+		if err != nil {
+			t.Fatalf("MkdirAll: %v", err)
+		}
+	}
+
+	return root
+}
+
+func TestIsDCFMountPoint(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]bool{
+		"DCIM":           true,
+		"PRIVATE/M4ROOT": true,
+		"M4ROOT":         false,
+		"PRIVATE":        false,
+	}
+
+	for directory, want := range cases {
+		got := isDCFMountPoint(newMountPoint(t, directory))
+		if got != want {
+			t.Errorf("isDCFMountPoint with only %q = %v, want %v",
+				directory, got, want)
+		}
+	}
+}
+
+func TestIsDCFMountPointIgnoresPlainFile(t *testing.T) {
+	t.Parallel()
+
+	root := t.TempDir()
+
+	err := os.WriteFile(filepath.Join(root, "DCIM"), nil, 0o600)
+	if err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+
+	if isDCFMountPoint(root) {
+		t.Fatal("isDCFMountPoint counted a plain file named DCIM as a directory")
+	}
+}
+
+func TestFilterDCFMountPointsReturnsRequestedCount(t *testing.T) {
+	t.Parallel()
+
+	first := newMountPoint(t, "DCIM")
+	second := newMountPoint(t, "PRIVATE/M4ROOT")
+	third := newMountPoint(t, "DCIM")
+
+	// This one holds no DCF store, so it must not use up the requested count.
+	mountpoints := []string{newMountPoint(t), first, second, third}
+
+	cases := map[int][]string{
+		0: {first, second, third},
+		1: {first},
+		2: {first, second},
+		3: {first, second, third},
+		4: {first, second, third},
+	}
+
+	for requestedCount, want := range cases {
+		got := filterDCFMountPoints(mountpoints, requestedCount)
+		if !slices.Equal(got, want) {
+			t.Errorf("filterDCFMountPoints(%d) = %v, want %v",
+				requestedCount, got, want)
+		}
+	}
+}