andrew/libakstdlib
1
0
Fork 0
Code Issues 26 Pull Requests Actions Packages Projects Releases Wiki Activity

No directory-reading wrapper: opendir, readdir, closedir #10

New Issue
Open
opened 2026-08-02 18:53:23 -04:00 by tachikoma · 2 comments
tachikoma commented 2026-08-02 18:53:23 -04:00
Collaborator
Copy Link

Source: TODO.md §4.1 and "Requested by consumers" (at 669b2b3)

There is no aksl_opendir / aksl_readdir / aksl_closedir, so a consumer
that wants to list a directory has to call opendir(3) itself and step outside
the error convention every other call in its file follows -- reporting through
errno where everything around it reports through an akerr_ErrorContext *.

This one has a consumer asking for it. akbasic hit it implementing Commodore
BASIC's DIRECTORY verb, and refuses the verb rather than working around it:
src/runtime_disk.c reports "DIRECTORY is not implemented: libakstdlib has no
directory-reading wrapper yet".

The shape it wants is the one the aksl_f* family already has -- a handle out
through a pointer parameter, NULL on success, ENOENT and EACCES propagated
as themselves.

readdir(3) returning NULL for both "end of directory" and "error, check
errno" is the same conflation aksl_fgetc and aksl_fgets already untangle,
and should be untangled the same way: AKERR_EOF for the end, the errno for the
error.

Full surface: opendir, fdopendir, readdir, closedir, rewinddir,
scandir.

Tests would want: a directory that does not exist, one that cannot be
searched, an empty one, and one whose entries outlast a single read -- plus
whatever the d_type portability story turns out to be, since not every
filesystem fills it in.

Files: src/, include/akstdlib.h; consumer: akbasic/src/runtime_disk.c

Filed by Tachikoma (Claude Code, Opus 5, 1M context)

**Source:** TODO.md §4.1 and "Requested by consumers" (at 669b2b3) There is no `aksl_opendir` / `aksl_readdir` / `aksl_closedir`, so **a consumer that wants to list a directory has to call `opendir(3)` itself and step outside the error convention every other call in its file follows** -- reporting through `errno` where everything around it reports through an `akerr_ErrorContext *`. **This one has a consumer asking for it.** `akbasic` hit it implementing Commodore BASIC's `DIRECTORY` verb, and **refuses the verb rather than working around it**: `src/runtime_disk.c` reports "DIRECTORY is not implemented: libakstdlib has no directory-reading wrapper yet". **The shape it wants** is the one the `aksl_f*` family already has -- a handle out through a pointer parameter, `NULL` on success, `ENOENT` and `EACCES` propagated as themselves. **`readdir(3)` returning NULL for both "end of directory" and "error, check errno" is the same conflation `aksl_fgetc` and `aksl_fgets` already untangle**, and should be untangled the same way: `AKERR_EOF` for the end, the errno for the error. Full surface: `opendir`, `fdopendir`, `readdir`, `closedir`, `rewinddir`, `scandir`. **Tests would want:** a directory that does not exist, one that cannot be searched, an empty one, and one whose entries outlast a single read -- plus whatever the `d_type` portability story turns out to be, since not every filesystem fills it in. **Files:** `src/`, `include/akstdlib.h`; consumer: `akbasic/src/runtime_disk.c` --- Filed by Tachikoma (Claude Code, Opus 5, 1M context)
tachikoma added this to the 0.3.0 milestone 2026-08-02 18:53:23 -04:00
tachikoma added the api-gapblast-radius:highstatus::grooming labels 2026-08-02 18:53:23 -04:00
andrew commented 2026-08-02 23:02:37 -04:00
Owner
Copy Link

@tachikoma let's get a better plan on this

@tachikoma let's get a better plan on this
tachikoma commented 2026-08-02 23:08:23 -04:00
Author
Collaborator
Copy Link

Plan

This one has a consumer and a settled shape. What is left is four decisions, three
of which I think are already answered by conventions the library follows
elsewhere. Steps:

  • Decide how an entry reaches the caller
  • Decide what the handle is
  • Untangle readdir(3)'s NULL
  • Split scandir out — it is blocked on something else
  • Say what d_type is allowed to mean
  • Build the test list, including the one that cannot run as root

How an entry reaches the caller

readdir(3) returns a pointer into the DIR's own storage, invalidated by the
next readdir or closedir on that handle. Two shapes:

Shape Cost
Hand back the struct dirent * Free, but the caller now holds a pointer with a lifetime rule that nothing in the type system enforces. Two interleaved reads of one handle silently alias.
Copy into a caller-supplied struct dirent * One struct copy per entry, and it obeys caller-owns-the-storage, which is the rule the hash map and aksl_fgets already follow.

Recommendation: copy into caller-supplied storage. struct dirent is a
complete type, so *dest = *ent is well-formed. The caveat to write down rather
than discover: POSIX leaves d_name's array size unspecified, so the copy is
portable in the sense that it compiles and copies everything the platform
declares, but a platform that declared d_name[1] and over-allocated would
truncate. Linux, the BSDs and macOS all declare it fixed. If that ever stops
being true the failure is silent, so it belongs in TODO.md next to the other
scope decisions.

The handle

DIR * passes through opaquely, exactly as FILE * does in aksl_fopen. No
aksl_Dir wrapper. There is nothing to add to it and wrapping it would break
dirfd(3) for anybody who needs it.

readdir's NULL

The same untangling aksl_fgetc already does, in the same order:

akerr_ErrorContext AKERR_NOIGNORE *aksl_readdir(DIR *dirp, struct dirent *dest)
{
    struct dirent *ent = NULL;
    PREPARE_ERROR(e);
    FAIL_ZERO_RETURN(e, dirp, AKERR_NULLPOINTER, "dirp=%p, dest=%p", (void *)dirp, (void *)dest);
    FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "dirp=%p, dest=%p", (void *)dirp, (void *)dest);
    errno = 0;
    ent = readdir(dirp);
    if ( ent == NULL ) {
	FAIL_NONZERO_RETURN(e, errno, AKSL_ERRNO_OR(AKERR_IO), "readdir failed");
	FAIL_RETURN(e, AKERR_EOF, "end of directory");
    }
    *dest = *ent;
    SUCCEED_RETURN(e);
}

Note this is the one place in the library where errno is the discriminator
and not just the status, because readdir has no ferror() equivalent. That is
worth a comment at the site — a later reader will otherwise "fix" it to match
aksl_fgetc exactly and lose the distinction.

End of directory is AKERR_EOF, not success. "Finding nothing is success"
governs searching functions. A read loop terminates on a status, the way the
aksl_fgetc and aksl_getline loops already do.

scandir should not land here

scandir(3) takes a filter callback and a comparison callback, and neither can
raise an akerr_ErrorContext * through a libc-defined signature. That is the
identical unsolved problem as #14 (qsort/bsearch want an akerror-aware
comparator, not a wrapper). It also allocates the array and every entry with
malloc, so it needs a matching aksl_scandir_free.

Recommendation: drop scandir from this issue and let it land with whatever
#14 settles. Wrapping it here would either invent a callback convention that #14
then has to contradict, or ship a wrapper whose callbacks cannot report. The
remaining surface — opendir, fdopendir, readdir, closedir, rewinddir —
has no such problem and unblocks the consumer today.

rewinddir(3) returns void and cannot fail, so its wrapper can only ever raise
AKERR_NULLPOINTER. Wrap it anyway: the NULL check is real, and a gap in the
family is worse than a thin function.

What d_type is allowed to mean

d_type is DT_UNKNOWN on filesystems that do not carry the type in the
directory entry. A caller that trusts it will work on ext4 and fail on the first
filesystem that does not fill it in, which is exactly the kind of bug that shows
up only in somebody else's deployment.

The header must say: d_type may be DT_UNKNOWN, and a caller that needs the
type must fall back to stat. That makes #9 a soft dependency — not for
compilation, but because the documented fallback should point at a wrapper that
exists rather than at stat(2) directly.

Tests

Case Asserts
opendir on a path that does not exist ENOENT propagated as itself
opendir on a directory with no search permission EACCES propagated as itself
opendir on a regular file ENOTDIR
Empty directory Exactly . and .., then AKERR_EOF
Directory with entries outlasting one read Every entry seen once; the copy survives the next readdir
fdopendir from an open fd Same entries as opendir on the path
rewinddir mid-walk The walk restarts
NULL dirp, NULL dest AKERR_NULLPOINTER on each

The EACCES case does not work as root, which is how containers usually run
CI. chmod 000 is ignored for a process with CAP_DAC_OVERRIDE, so that test
would pass by accident locally and fail in CI, or the reverse. It needs a
geteuid() == 0 guard that skips it, and the skip must be visible in the CTest
output rather than silently returning zero — otherwise the suite reports a
passing test that never ran.

The "entries outlast a single read" case is the one that actually justifies the
copy decision above. Write it so it fails if the implementation hands back the
library's pointer: read entry one, keep it, read entry two, then assert entry
one's d_name is still the first name.

Files

include/akstdlib.h (declarations plus the full Doxygen block — the docs
target fails on a missing @throws), a new src/dir.c rather than growing
src/stream.c, its row in the AGENTS.md file table, tests/test_dir.c plus its
entry in AKSL_TESTS, and the consumer change in
akbasic/src/runtime_disk.c:399 that deletes the refusal and implements
DIRECTORY.

What I would change on the ticket

Scope is settled if you take the copy-into-caller-storage decision and the
scandir split. That moves it off status::grooming. The akbasic side is a
separate issue against akbasic, since it lands after this ships.

— Tachikoma (Claude Code, Opus 5, 1M context)

## Plan This one has a consumer and a settled shape. What is left is four decisions, three of which I think are already answered by conventions the library follows elsewhere. Steps: - Decide how an entry reaches the caller - Decide what the handle is - Untangle `readdir(3)`'s NULL - Split `scandir` out — it is blocked on something else - Say what `d_type` is allowed to mean - Build the test list, including the one that cannot run as root ### How an entry reaches the caller `readdir(3)` returns a pointer into the `DIR`'s own storage, invalidated by the next `readdir` or `closedir` on that handle. Two shapes: | Shape | Cost | |---|---| | Hand back the `struct dirent *` | Free, but the caller now holds a pointer with a lifetime rule that nothing in the type system enforces. Two interleaved reads of one handle silently alias. | | Copy into a caller-supplied `struct dirent *` | One struct copy per entry, and it obeys **caller-owns-the-storage**, which is the rule the hash map and `aksl_fgets` already follow. | **Recommendation: copy into caller-supplied storage.** `struct dirent` is a complete type, so `*dest = *ent` is well-formed. The caveat to write down rather than discover: POSIX leaves `d_name`'s array size unspecified, so the copy is portable in the sense that it compiles and copies everything the platform declares, but a platform that declared `d_name[1]` and over-allocated would truncate. Linux, the BSDs and macOS all declare it fixed. If that ever stops being true the failure is silent, so it belongs in `TODO.md` next to the other scope decisions. ### The handle `DIR *` passes through opaquely, exactly as `FILE *` does in `aksl_fopen`. No `aksl_Dir` wrapper. There is nothing to add to it and wrapping it would break `dirfd(3)` for anybody who needs it. ### `readdir`'s NULL The same untangling `aksl_fgetc` already does, in the same order: ```c akerr_ErrorContext AKERR_NOIGNORE *aksl_readdir(DIR *dirp, struct dirent *dest) { struct dirent *ent = NULL; PREPARE_ERROR(e); FAIL_ZERO_RETURN(e, dirp, AKERR_NULLPOINTER, "dirp=%p, dest=%p", (void *)dirp, (void *)dest); FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "dirp=%p, dest=%p", (void *)dirp, (void *)dest); errno = 0; ent = readdir(dirp); if ( ent == NULL ) { FAIL_NONZERO_RETURN(e, errno, AKSL_ERRNO_OR(AKERR_IO), "readdir failed"); FAIL_RETURN(e, AKERR_EOF, "end of directory"); } *dest = *ent; SUCCEED_RETURN(e); } ``` Note this is the one place in the library where `errno` is the *discriminator* and not just the status, because `readdir` has no `ferror()` equivalent. That is worth a comment at the site — a later reader will otherwise "fix" it to match `aksl_fgetc` exactly and lose the distinction. **End of directory is `AKERR_EOF`, not success.** "Finding nothing is success" governs *searching* functions. A read loop terminates on a status, the way the `aksl_fgetc` and `aksl_getline` loops already do. ### `scandir` should not land here `scandir(3)` takes a filter callback and a comparison callback, and neither can raise an `akerr_ErrorContext *` through a libc-defined signature. That is the identical unsolved problem as **#14** (`qsort`/`bsearch` want an akerror-aware comparator, not a wrapper). It also allocates the array and every entry with `malloc`, so it needs a matching `aksl_scandir_free`. **Recommendation: drop `scandir` from this issue** and let it land with whatever #14 settles. Wrapping it here would either invent a callback convention that #14 then has to contradict, or ship a wrapper whose callbacks cannot report. The remaining surface — `opendir`, `fdopendir`, `readdir`, `closedir`, `rewinddir` — has no such problem and unblocks the consumer today. `rewinddir(3)` returns `void` and cannot fail, so its wrapper can only ever raise `AKERR_NULLPOINTER`. Wrap it anyway: the NULL check is real, and a gap in the family is worse than a thin function. ### What `d_type` is allowed to mean `d_type` is `DT_UNKNOWN` on filesystems that do not carry the type in the directory entry. A caller that trusts it will work on ext4 and fail on the first filesystem that does not fill it in, which is exactly the kind of bug that shows up only in somebody else's deployment. The header must say: **`d_type` may be `DT_UNKNOWN`, and a caller that needs the type must fall back to `stat`.** That makes **#9** a soft dependency — not for compilation, but because the documented fallback should point at a wrapper that exists rather than at `stat(2)` directly. ### Tests | Case | Asserts | |---|---| | `opendir` on a path that does not exist | `ENOENT` propagated as itself | | `opendir` on a directory with no search permission | `EACCES` propagated as itself | | `opendir` on a regular file | `ENOTDIR` | | Empty directory | Exactly `.` and `..`, then `AKERR_EOF` | | Directory with entries outlasting one read | Every entry seen once; the copy survives the next `readdir` | | `fdopendir` from an `open` fd | Same entries as `opendir` on the path | | `rewinddir` mid-walk | The walk restarts | | NULL `dirp`, NULL `dest` | `AKERR_NULLPOINTER` on each | **The `EACCES` case does not work as root**, which is how containers usually run CI. `chmod 000` is ignored for a process with `CAP_DAC_OVERRIDE`, so that test would pass by accident locally and fail in CI, or the reverse. It needs a `geteuid() == 0` guard that skips it, and the skip must be visible in the CTest output rather than silently returning zero — otherwise the suite reports a passing test that never ran. The "entries outlast a single read" case is the one that actually justifies the copy decision above. Write it so it fails if the implementation hands back the library's pointer: read entry one, keep it, read entry two, then assert entry one's `d_name` is still the first name. ### Files `include/akstdlib.h` (declarations plus the full Doxygen block — the `docs` target fails on a missing `@throws`), a new `src/dir.c` rather than growing `src/stream.c`, its row in the AGENTS.md file table, `tests/test_dir.c` plus its entry in `AKSL_TESTS`, and the consumer change in `akbasic/src/runtime_disk.c:399` that deletes the refusal and implements `DIRECTORY`. ### What I would change on the ticket Scope is settled if you take the copy-into-caller-storage decision and the `scandir` split. That moves it off `status::grooming`. The akbasic side is a separate issue against akbasic, since it lands after this ships. --- *— Tachikoma (Claude Code, Opus 5, 1M context)*
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
abi-break
Cannot land without a soname bump
 api-gap
A caller cannot express something
 blast-radius:high
Ordering: highest blast radius
 blast-radius:low
Ordering: lowest blast radius
 blast-radius:medium
Ordering: middling blast radius
 defect
Wrong behaviour reachable from the public API
 design-decision
Needs a decision written down before code
 docs
Documentation, comments, AGENTS.md
 hygiene
Style, dead code, consistency
 packaging
Build, install, pkg-config, CMake config
 performance
Has a measurement behind it
 status::blocked
Waiting on something else -- a decision, a dependency, or another issue
 status::grooming
Not yet worked out: scope, approach or priority still being settled
 status::ready
Understood well enough to start: scope and approach are settled
 test-coverage
A gap in the suites, not a defect
No Label api-gap blast-radius:high status::grooming
Milestone
No items
No Milestone 0.3.0
Projects
Clear projects
No project
Assignees
Clear assignees
tachikoma andrew
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: andrew/libakstdlib#10
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?