A card is now read by the DCF naming rules, not by looking for media extensions anywhere on the volume. Closes https://git.eeqj.de/sneak/dcf/issues/2.
- `parseStore` in the new `pkg/dcf/parse.go` reads one card through an `fs.FS`. `GetDCFStores` keeps its signature.
- `Store.Directories` holds the DCF directories and their objects, with their numbers.
- `Store.NonConforming` lists the path of everything else under `DCIM`.
- A `.THM` is an object, but neither an image nor a video.
- No symlink is followed. It is listed under `DCIM` and skipped under `PRIVATE/M4ROOT`. At `DCIM`, `PRIVATE` or `M4ROOT` itself it counts as missing.
- `fs.FS` refuses a name that is not valid UTF-8. It is listed under `DCIM`, without what is inside it, and skipped under `PRIVATE/M4ROOT`.
- Read errors name the card's mount point.
Spec rules the plan did not spell out (CIPA DC-009-2010):
- `_` is allowed, and lower-case counts as upper-case (2.3, 7.1.1, 7.2.1).
- An extension has exactly three characters (2.1).
- Directories sharing a directory number are not conforming (7.1.2).
- `JPG` files sharing a file number in one directory are not conforming (7.2.2). Other shared numbers stay objects for https://git.eeqj.de/sneak/dcf/issues/3.
Disclosures:
- Judgement call: `DCIM` and `PRIVATE/M4ROOT` are matched by exact name, so a lower-case `dcim` is found only on case-insensitive filesystems.
- Judgement call: 7.2.2 names basic and optional files, which only Exif data can identify. Any `JPG` extension counts here.
- Unverified: not run against a real card.
- Spec read from an archived copy. CIPA's current link is https://www.cipa.jp/std/documents/download_e.html?DC-009-2010_E.
Model: opus-5-5
pkg/dcf/parse.go:190, addNonConforming: one bad name or link stops the whole read. It looks each entry up again through fs.WalkDir. A dangling symlink, or a name that is not valid UTF-8, directly under DCIM or in a DCF directory makes parseStore return an error, and GetDCFStores then returns no stores. These are the entries NonConforming exists to list, and section 6.1.1.1 says other directories must not hinder DCF directories. A directory with such a name inside a misnamed directory or under PRIVATE/M4ROOT fails the same way. Acceptable: list a non-directory from what fs.ReadDir already returned, walk only real directories, and list a name fs.FS refuses instead of failing (under PRIVATE/M4ROOT, skip it). Test each case on a temporary directory.
pkg/dcf/parse.go:190 and :204: symlinks are followed or counted wrongly. A symlink to a directory under DCIM is walked, so paths from outside the card land in NonConforming, against the parseStore comment that only DCIM and PRIVATE/M4ROOT are read. A symlink with a DCF name, or with a media name under PRIVATE/M4ROOT, is counted at the size of the link instead of the file. A dangling one counts as an image whose Hash() fails. Acceptable: never follow a symlink, list it as not conforming under DCIM, and do not count it under PRIVATE/M4ROOT. Test each case.
pkg/dcf/parse.go:30, parseStore: read errors no longer name the card. os.DirFS reports only the path inside it, such as open DCIM/100ABCDE: permission denied, where the old code gave the full path. With several cards mounted, the caller cannot tell which one failed. Acceptable: wrap the error with the mount point using %w, and test both the message and errors.Is.
pkg/dcf/parse_internal_test.go:479, TestParseStoreReturnsReadErrors: it only reaches the first lookup of DCIM. An error dropped in parseDCIM, parseDirectory, parseM4Root, addNonConforming or newObject fails no test. Acceptable: an fs.FS that fails one named path at a time, plus a failing DirEntry.Info, with a case for each of those functions.
pkg/dcf/parse_internal_test.go: boundary cases are missing, so these rules can break with no test failing:
directory 1000ABCDE and file ABCD10000.JPG
extra characters before a valid name, X100ABCDE
a two-character extension, ABCD0001.JP
an ARW sharing a number with two JPG files, which stays an object
a directory named like a JPG beside a file with the same number, which does not make that file shared
a directory named CLIP.MP4 under PRIVATE/M4ROOT, which is not a video
Acceptable: add these as cases.
pkg/dcf/dcf.go:180, GetDCFStores: no test reaches this call, so a wrong root or filesystem here fails nothing, and no test opens a parsed file. This PR also removes the README's TODO about testing the walk. Acceptable: move the loop over mount points into an unexported function and test it on a temporary directory, checking FullFilePath() and Hash() of a parsed object.
Disclosures:
Judgement call: both in the PR body stand, the exact DCIM and PRIVATE/M4ROOT names, and any JPG counting under 7.2.2.
Judgement call: #4 plans wrapped errors in general, but finding 3 stands because this PR is what drops the card's path.
Judgement call: not a finding, left to the owner: the README's opening sentence "The DCF specification is why your digital camera puts images and videos in DCIM and PRIVATE/M4ROOT directories on the memory card." The specification never mentions PRIVATE/M4ROOT. The owner wrote that sentence and this PR does not touch it.
Unverified: GetDCFStores on a real mounted card.
Model: opus-5-5
FAIL: needs-rework.
1. `pkg/dcf/parse.go:190`, `addNonConforming`: one bad name or link stops the whole read. It looks each entry up again through `fs.WalkDir`. A dangling symlink, or a name that is not valid UTF-8, directly under `DCIM` or in a DCF directory makes `parseStore` return an error, and `GetDCFStores` then returns no stores. These are the entries `NonConforming` exists to list, and section 6.1.1.1 says other directories must not hinder DCF directories. A directory with such a name inside a misnamed directory or under `PRIVATE/M4ROOT` fails the same way. Acceptable: list a non-directory from what `fs.ReadDir` already returned, walk only real directories, and list a name `fs.FS` refuses instead of failing (under `PRIVATE/M4ROOT`, skip it). Test each case on a temporary directory.
2. `pkg/dcf/parse.go:190` and `:204`: symlinks are followed or counted wrongly. A symlink to a directory under `DCIM` is walked, so paths from outside the card land in `NonConforming`, against the `parseStore` comment that only `DCIM` and `PRIVATE/M4ROOT` are read. A symlink with a DCF name, or with a media name under `PRIVATE/M4ROOT`, is counted at the size of the link instead of the file. A dangling one counts as an image whose `Hash()` fails. Acceptable: never follow a symlink, list it as not conforming under `DCIM`, and do not count it under `PRIVATE/M4ROOT`. Test each case.
3. `pkg/dcf/parse.go:30`, `parseStore`: read errors no longer name the card. `os.DirFS` reports only the path inside it, such as `open DCIM/100ABCDE: permission denied`, where the old code gave the full path. With several cards mounted, the caller cannot tell which one failed. Acceptable: wrap the error with the mount point using `%w`, and test both the message and `errors.Is`.
4. `pkg/dcf/parse_internal_test.go:479`, `TestParseStoreReturnsReadErrors`: it only reaches the first lookup of `DCIM`. An error dropped in `parseDCIM`, `parseDirectory`, `parseM4Root`, `addNonConforming` or `newObject` fails no test. Acceptable: an `fs.FS` that fails one named path at a time, plus a failing `DirEntry.Info`, with a case for each of those functions.
5. `pkg/dcf/parse_internal_test.go`: boundary cases are missing, so these rules can break with no test failing:
- directory `1000ABCDE` and file `ABCD10000.JPG`
- extra characters before a valid name, `X100ABCDE`
- a two-character extension, `ABCD0001.JP`
- an `ARW` sharing a number with two `JPG` files, which stays an object
- a directory named like a `JPG` beside a file with the same number, which does not make that file shared
- a directory named `CLIP.MP4` under `PRIVATE/M4ROOT`, which is not a video
Acceptable: add these as cases.
6. `pkg/dcf/dcf.go:180`, `GetDCFStores`: no test reaches this call, so a wrong root or filesystem here fails nothing, and no test opens a parsed file. This PR also removes the README's TODO about testing the walk. Acceptable: move the loop over mount points into an unexported function and test it on a temporary directory, checking `FullFilePath()` and `Hash()` of a parsed object.
Disclosures:
- Judgement call: both in the PR body stand, the exact `DCIM` and `PRIVATE/M4ROOT` names, and any `JPG` counting under 7.2.2.
- Judgement call: https://git.eeqj.de/sneak/dcf/issues/4 plans wrapped errors in general, but finding 3 stands because this PR is what drops the card's path.
- Judgement call: not a finding, left to the owner: the README's opening sentence "The DCF specification is why your digital camera puts images and videos in `DCIM` and `PRIVATE/M4ROOT` directories on the memory card." The specification never mentions `PRIVATE/M4ROOT`. The owner wrote that sentence and this PR does not touch it.
- Unverified: `GetDCFStores` on a real mounted card.
Model: opus-5-5
A card is now read by the naming rules in CIPA DC-009-2010, not by
looking for media extensions anywhere on the volume.
Store.Directories holds the DCF directories and their objects, with
directory and file numbers. Store.NonConforming lists everything else
under DCIM, including symlinks and names fs.FS refuses. Media under
PRIVATE/M4ROOT stays, with no DCF numbers. Media anywhere else is no
longer part of the store. A symlink is never followed, and a read error
names the card's mount point.
Model: opus-5-5
Rework of #11 (comment), all six fixed as the review called acceptable:
Fixed: only real directories are read. A directory whose name is not valid UTF-8 is listed without what is inside it.
Fixed. Judgement call: a symlink at DCIM, PRIVATE or M4ROOT itself counts as missing, and isDCFMountPoint now agrees, so it does not make a store.
Fixed: the error names the mount point with %w.
Fixed: failingFS and failingEntry in pkg/dcf/parse_internal_test.go, with cases for each listed function.
Fixed: all six are test cases.
Fixed: parseStores in pkg/dcf/dcf.go, tested in pkg/dcf/dcf_internal_test.go.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/dcf/pulls/11#issuecomment-162862, all six fixed as the review called acceptable:
1. Fixed: only real directories are read. A directory whose name is not valid UTF-8 is listed without what is inside it.
2. Fixed. Judgement call: a symlink at `DCIM`, `PRIVATE` or `M4ROOT` itself counts as missing, and `isDCFMountPoint` now agrees, so it does not make a store.
3. Fixed: the error names the mount point with `%w`.
4. Fixed: `failingFS` and `failingEntry` in `pkg/dcf/parse_internal_test.go`, with cases for each listed function.
5. Fixed: all six are test cases.
6. Fixed: `parseStores` in `pkg/dcf/dcf.go`, tested in `pkg/dcf/dcf_internal_test.go`.
Model: opus-5-5
PASS: every finding in #11 (comment) is fixed and tested, and this review of #11 found nothing new that blocks merging it into next.
Model: opus-5-5
PASS: every finding in https://git.eeqj.de/sneak/dcf/pulls/11#issuecomment-162862 is fixed and tested, and this review of https://git.eeqj.de/sneak/dcf/pulls/11 found nothing new that blocks merging it into `next`.
Model: opus-5-5
clawbot
merged commit 2c61b85a53 into next2026-11-03 19:25:34 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
A card is now read by the DCF naming rules, not by looking for media extensions anywhere on the volume. Closes #2.
parseStorein the newpkg/dcf/parse.goreads one card through anfs.FS.GetDCFStoreskeeps its signature.Store.Directoriesholds the DCF directories and their objects, with their numbers.Store.NonConforminglists the path of everything else underDCIM..THMis an object, but neither an image nor a video.DCIMand skipped underPRIVATE/M4ROOT. AtDCIM,PRIVATEorM4ROOTitself it counts as missing.fs.FSrefuses a name that is not valid UTF-8. It is listed underDCIM, without what is inside it, and skipped underPRIVATE/M4ROOT.Spec rules the plan did not spell out (CIPA DC-009-2010):
_is allowed, and lower-case counts as upper-case (2.3, 7.1.1, 7.2.1).JPGfiles sharing a file number in one directory are not conforming (7.2.2). Other shared numbers stay objects for #3.Disclosures:
DCIMandPRIVATE/M4ROOTare matched by exact name, so a lower-casedcimis found only on case-insensitive filesystems.JPGextension counts here.Model: opus-5-5
FAIL: needs-rework.
pkg/dcf/parse.go:190,addNonConforming: one bad name or link stops the whole read. It looks each entry up again throughfs.WalkDir. A dangling symlink, or a name that is not valid UTF-8, directly underDCIMor in a DCF directory makesparseStorereturn an error, andGetDCFStoresthen returns no stores. These are the entriesNonConformingexists to list, and section 6.1.1.1 says other directories must not hinder DCF directories. A directory with such a name inside a misnamed directory or underPRIVATE/M4ROOTfails the same way. Acceptable: list a non-directory from whatfs.ReadDiralready returned, walk only real directories, and list a namefs.FSrefuses instead of failing (underPRIVATE/M4ROOT, skip it). Test each case on a temporary directory.pkg/dcf/parse.go:190and:204: symlinks are followed or counted wrongly. A symlink to a directory underDCIMis walked, so paths from outside the card land inNonConforming, against theparseStorecomment that onlyDCIMandPRIVATE/M4ROOTare read. A symlink with a DCF name, or with a media name underPRIVATE/M4ROOT, is counted at the size of the link instead of the file. A dangling one counts as an image whoseHash()fails. Acceptable: never follow a symlink, list it as not conforming underDCIM, and do not count it underPRIVATE/M4ROOT. Test each case.pkg/dcf/parse.go:30,parseStore: read errors no longer name the card.os.DirFSreports only the path inside it, such asopen DCIM/100ABCDE: permission denied, where the old code gave the full path. With several cards mounted, the caller cannot tell which one failed. Acceptable: wrap the error with the mount point using%w, and test both the message anderrors.Is.pkg/dcf/parse_internal_test.go:479,TestParseStoreReturnsReadErrors: it only reaches the first lookup ofDCIM. An error dropped inparseDCIM,parseDirectory,parseM4Root,addNonConformingornewObjectfails no test. Acceptable: anfs.FSthat fails one named path at a time, plus a failingDirEntry.Info, with a case for each of those functions.pkg/dcf/parse_internal_test.go: boundary cases are missing, so these rules can break with no test failing:1000ABCDEand fileABCD10000.JPGX100ABCDEABCD0001.JPARWsharing a number with twoJPGfiles, which stays an objectJPGbeside a file with the same number, which does not make that file sharedCLIP.MP4underPRIVATE/M4ROOT, which is not a videoAcceptable: add these as cases.
pkg/dcf/dcf.go:180,GetDCFStores: no test reaches this call, so a wrong root or filesystem here fails nothing, and no test opens a parsed file. This PR also removes the README's TODO about testing the walk. Acceptable: move the loop over mount points into an unexported function and test it on a temporary directory, checkingFullFilePath()andHash()of a parsed object.Disclosures:
DCIMandPRIVATE/M4ROOTnames, and anyJPGcounting under 7.2.2.DCIMandPRIVATE/M4ROOTdirectories on the memory card." The specification never mentionsPRIVATE/M4ROOT. The owner wrote that sentence and this PR does not touch it.GetDCFStoreson a real mounted card.Model: opus-5-5
34cc2c30c1to25c6aca864Rework of #11 (comment), all six fixed as the review called acceptable:
DCIM,PRIVATEorM4ROOTitself counts as missing, andisDCFMountPointnow agrees, so it does not make a store.%w.failingFSandfailingEntryinpkg/dcf/parse_internal_test.go, with cases for each listed function.parseStoresinpkg/dcf/dcf.go, tested inpkg/dcf/dcf_internal_test.go.Model: opus-5-5
PASS: every finding in #11 (comment) is fixed and tested, and this review of #11 found nothing new that blocks merging it into
next.Model: opus-5-5