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

Wrap sys/stat.h: stat, fstat, lstat, fstatat, statvfs, fstatvfs #9

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

Source: TODO.md §4.1 (at 669b2b3)

  • stat, fstat, lstat, fstatat
  • statvfs, fstatvfs

Straightforward errno-reporting wrappers; the interest is in whether the wrapper
hands back the caller's own struct stat (consistent with the
caller-owns-the-storage rule the hash map already follows) or a subset the library
defines.

Files: src/, include/akstdlib.h

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

**Source:** TODO.md §4.1 (at 669b2b3) - [ ] `stat`, `fstat`, `lstat`, `fstatat` - [ ] `statvfs`, `fstatvfs` Straightforward errno-reporting wrappers; the interest is in whether the wrapper hands back the caller's own `struct stat` (consistent with the caller-owns-the-storage rule the hash map already follows) or a subset the library defines. **Files:** `src/`, `include/akstdlib.h` --- Filed by Tachikoma (Claude Code, Opus 5, 1M context)
tachikoma added this to the 0.3.0 milestone 2026-08-02 18:53:22 -04:00
tachikoma added the api-gapblast-radius:mediumstatus::grooming labels 2026-08-02 18:53:22 -04:00
andrew commented 2026-08-02 23:02:07 -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:22 -04:00
Author
Collaborator
Copy Link

Plan

The open question on the ticket — caller's struct stat or a subset this library
defines — is answered by a rule the library already follows. The rest is scope.
Steps:

  • Settle the struct question
  • Settle what ENOENT means here
  • Decide whether the rest of sys/stat.h comes along
  • Handle the fstatat flags
  • Build the test list

The struct question

Hand back the caller's own struct stat. Three reasons, in blast-radius
order:

  1. It is the caller-owns-the-storage rule, which the hash map and the
    aksl_str* family already follow. A subset type would be the only allocated
    output in the file-metadata surface.
  2. A subset has to be maintained. Every field this library chooses not to carry
    is a caller that has to call stat(2) directly and step outside the error
    convention — which is the exact complaint that opened #10.
  3. struct stat is POSIX and stable. There is no portability win to buy.

So the shape is the one the rest of the library uses — output through a pointer
parameter, NULL on success:

akerr_ErrorContext AKERR_NOIGNORE *aksl_stat(const char *pathname, struct stat *dest);
akerr_ErrorContext AKERR_NOIGNORE *aksl_lstat(const char *pathname, struct stat *dest);
akerr_ErrorContext AKERR_NOIGNORE *aksl_fstat(int fd, struct stat *dest);
akerr_ErrorContext AKERR_NOIGNORE *aksl_fstatat(int dirfd, const char *pathname, struct stat *dest, int flags);
akerr_ErrorContext AKERR_NOIGNORE *aksl_statvfs(const char *path, struct statvfs *dest);
akerr_ErrorContext AKERR_NOIGNORE *aksl_fstatvfs(int fd, struct statvfs *dest);

This costs the header a public #include <sys/stat.h> and <sys/statvfs.h>.
akstdlib.h already publishes <sys/types.h> alongside <stdarg.h>,
<stddef.h>, <stdint.h> and <stdio.h>, so this is a widening of an existing
POSIX dependency rather than a new one — but it is still a widening, and every
consumer of akstdlib.h picks it up whether or not it calls these.

ENOENT is a failure

Worth writing down, because the library has a rule that looks like it says
otherwise. "Finding nothing is success" governs searching functions —
aksl_strchr writes NULL and returns NULL. stat is not a search. The caller
named one path and asked about it, so ENOENT propagates as itself and a caller
testing for existence writes a DETECT on ENOENT.

aksl_fopen already behaves this way for a missing file. Anything else would put
two file-facing families on opposite conventions.

The rest of sys/stat.h

The header also declares mkdir, mkdirat, chmod, fchmod, fchmodat,
umask and mknod. The ticket covers only the stat family, and I think that
is right, but it should be a decision rather than an omission: those are
mutating calls and they belong with whatever wraps unlink/rename/rmdir,
not with metadata reads.

Recommendation: file a separate issue for the mutating sys/stat.h surface
and say so in TODO.md, so the next reader does not think the family was missed.

umask is a genuine oddity — it cannot fail and it returns the previous mask, so
its wrapper would raise nothing at all. That is the same "cannot fail" scope
question as rewinddir in #10, and both should be answered the same way.

fstatat flags

AT_SYMLINK_NOFOLLOW makes aksl_fstatat cover aksl_lstat, and AT_FDCWD
makes it cover aksl_stat. Wrap all four anyway — the two-argument spellings are
what callers actually write, and collapsing them would make the common case carry
two constants it does not care about.

Validate flags: anything outside AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_NO_AUTOMOUNT is AKERR_VALUE raised by the wrapper, before the call. EINVAL
from the kernel says only "bad flags" with no indication which.

Tests

Case Asserts
stat on a regular file st_size and S_ISREG match what was written
stat on a path that does not exist ENOENT
stat through a component that is not a directory ENOTDIR
stat vs lstat on a symlink One follows, one does not; S_ISLNK differs
fstat on an open fd Same st_ino as stat on its path
fstat on a closed fd EBADF
fstatat with AT_FDCWD Matches aksl_stat
fstatat with an unknown flag bit AKERR_VALUE, and errno untouched
statvfs on . f_frsize non-zero
NULL pathname, NULL dest AKERR_NULLPOINTER on each

The symlink cases need the test to create the symlink, which means the suite
needs symlink(2) — not wrapped, and not worth wrapping for a test. Call it
directly from the test and note why.

ENAMETOOLONG is reachable with a path over PATH_MAX if you want the branch,
but it is a libc-boundary check rather than library behavior. I would leave it
out and let the coverage listing show it.

Files

include/akstdlib.h, a new src/stat.c (or fold into the src/dir.c that #10
creates — they are the same domain and d_type's DT_UNKNOWN fallback lands in
stat), the AGENTS.md file table, tests/test_stat.c and its AKSL_TESTS
entry.

Sequencing

Land this with or before #10. #10's d_type documentation has to tell the
caller to fall back to stat, and that should name a wrapper that exists.

What I would change on the ticket

Scope settles if you take the caller's-struct decision and split the mutating
surface into its own issue. The umask-and-rewinddir "cannot fail" question is
the one thing that spans both tickets and wants a single answer.

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

## Plan The open question on the ticket — caller's `struct stat` or a subset this library defines — is answered by a rule the library already follows. The rest is scope. Steps: - Settle the struct question - Settle what `ENOENT` means here - Decide whether the rest of `sys/stat.h` comes along - Handle the `fstatat` flags - Build the test list ### The struct question **Hand back the caller's own `struct stat`.** Three reasons, in blast-radius order: 1. It is the **caller-owns-the-storage** rule, which the hash map and the `aksl_str*` family already follow. A subset type would be the only allocated output in the file-metadata surface. 2. A subset has to be maintained. Every field this library chooses not to carry is a caller that has to call `stat(2)` directly and step outside the error convention — which is the exact complaint that opened **#10**. 3. `struct stat` is POSIX and stable. There is no portability win to buy. So the shape is the one the rest of the library uses — output through a pointer parameter, `NULL` on success: ```c akerr_ErrorContext AKERR_NOIGNORE *aksl_stat(const char *pathname, struct stat *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_lstat(const char *pathname, struct stat *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_fstat(int fd, struct stat *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_fstatat(int dirfd, const char *pathname, struct stat *dest, int flags); akerr_ErrorContext AKERR_NOIGNORE *aksl_statvfs(const char *path, struct statvfs *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_fstatvfs(int fd, struct statvfs *dest); ``` This costs the header a public `#include <sys/stat.h>` and `<sys/statvfs.h>`. `akstdlib.h` already publishes `<sys/types.h>` alongside `<stdarg.h>`, `<stddef.h>`, `<stdint.h>` and `<stdio.h>`, so this is a widening of an existing POSIX dependency rather than a new one — but it is still a widening, and every consumer of `akstdlib.h` picks it up whether or not it calls these. ### `ENOENT` is a failure Worth writing down, because the library has a rule that looks like it says otherwise. **"Finding nothing is success" governs searching functions** — `aksl_strchr` writes NULL and returns NULL. `stat` is not a search. The caller named one path and asked about it, so `ENOENT` propagates as itself and a caller testing for existence writes a `DETECT` on `ENOENT`. `aksl_fopen` already behaves this way for a missing file. Anything else would put two file-facing families on opposite conventions. ### The rest of `sys/stat.h` The header also declares `mkdir`, `mkdirat`, `chmod`, `fchmod`, `fchmodat`, `umask` and `mknod`. The ticket covers only the `stat` family, and I think that is right, but it should be a decision rather than an omission: those are *mutating* calls and they belong with whatever wraps `unlink`/`rename`/`rmdir`, not with metadata reads. **Recommendation: file a separate issue for the mutating `sys/stat.h` surface** and say so in `TODO.md`, so the next reader does not think the family was missed. `umask` is a genuine oddity — it cannot fail and it returns the previous mask, so its wrapper would raise nothing at all. That is the same "cannot fail" scope question as `rewinddir` in #10, and both should be answered the same way. ### `fstatat` flags `AT_SYMLINK_NOFOLLOW` makes `aksl_fstatat` cover `aksl_lstat`, and `AT_FDCWD` makes it cover `aksl_stat`. Wrap all four anyway — the two-argument spellings are what callers actually write, and collapsing them would make the common case carry two constants it does not care about. Validate `flags`: anything outside `AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_NO_AUTOMOUNT` is `AKERR_VALUE` raised by the wrapper, before the call. `EINVAL` from the kernel says only "bad flags" with no indication which. ### Tests | Case | Asserts | |---|---| | `stat` on a regular file | `st_size` and `S_ISREG` match what was written | | `stat` on a path that does not exist | `ENOENT` | | `stat` through a component that is not a directory | `ENOTDIR` | | `stat` vs `lstat` on a symlink | One follows, one does not; `S_ISLNK` differs | | `fstat` on an open fd | Same `st_ino` as `stat` on its path | | `fstat` on a closed fd | `EBADF` | | `fstatat` with `AT_FDCWD` | Matches `aksl_stat` | | `fstatat` with an unknown flag bit | `AKERR_VALUE`, and `errno` untouched | | `statvfs` on `.` | `f_frsize` non-zero | | NULL `pathname`, NULL `dest` | `AKERR_NULLPOINTER` on each | The symlink cases need the test to create the symlink, which means the suite needs `symlink(2)` — not wrapped, and not worth wrapping for a test. Call it directly from the test and note why. `ENAMETOOLONG` is reachable with a path over `PATH_MAX` if you want the branch, but it is a libc-boundary check rather than library behavior. I would leave it out and let the coverage listing show it. ### Files `include/akstdlib.h`, a new `src/stat.c` (or fold into the `src/dir.c` that #10 creates — they are the same domain and `d_type`'s `DT_UNKNOWN` fallback lands in `stat`), the AGENTS.md file table, `tests/test_stat.c` and its `AKSL_TESTS` entry. ### Sequencing **Land this with or before #10.** #10's `d_type` documentation has to tell the caller to fall back to `stat`, and that should name a wrapper that exists. ### What I would change on the ticket Scope settles if you take the caller's-struct decision and split the mutating surface into its own issue. The `umask`-and-`rewinddir` "cannot fail" question is the one thing that spans both tickets and wants a single answer. --- *— Tachikoma (Claude Code, Opus 5, 1M context)*
andrew commented 2026-08-02 23:26:10 -04:00
Owner
Copy Link

@tachikoma

umask is a genuine oddity — it cannot fail and it returns the previous mask, so its wrapper would raise nothing at all.

This is incorrect. from man 2 uname:

SYNOPSIS
       #include <sys/utsname.h>

       int uname(struct utsname *buf);

DESCRIPTION
       uname() returns system information in the structure pointed to by buf.  The utsname struct is defined in <sys/utsname.h>:

           struct utsname {
               char sysname[];    /* Operating system name (e.g., "Linux") */
               char nodename[];   /* Name within communications network
                                     to which the node is attached, if any */
               char release[];    /* Operating system release
                                     (e.g., "2.6.28") */
               char version[];    /* Operating system version */
               char machine[];    /* Hardware type identifier */
           #ifdef _GNU_SOURCE
               char domainname[]; /* NIS or YP domain name */
           #endif
           };

       The length of the arrays in a struct utsname is unspecified (see NOTES); the fields are terminated by a null byte ('\0').

RETURN VALUE
       On success, zero is returned.  On error, -1 is returned, and errno is set to indicate the error.

ERRORS
       EFAULT buf is not valid.
@tachikoma > umask is a genuine oddity — it cannot fail and it returns the previous mask, so its wrapper would raise nothing at all. This is incorrect. from `man 2 uname`: ``` SYNOPSIS #include <sys/utsname.h> int uname(struct utsname *buf); DESCRIPTION uname() returns system information in the structure pointed to by buf. The utsname struct is defined in <sys/utsname.h>: struct utsname { char sysname[]; /* Operating system name (e.g., "Linux") */ char nodename[]; /* Name within communications network to which the node is attached, if any */ char release[]; /* Operating system release (e.g., "2.6.28") */ char version[]; /* Operating system version */ char machine[]; /* Hardware type identifier */ #ifdef _GNU_SOURCE char domainname[]; /* NIS or YP domain name */ #endif }; The length of the arrays in a struct utsname is unspecified (see NOTES); the fields are terminated by a null byte ('\0'). RETURN VALUE On success, zero is returned. On error, -1 is returned, and errno is set to indicate the error. ERRORS EFAULT buf is not valid. ```
andrew commented 2026-08-02 23:36:48 -04:00
Owner
Copy Link

@tachikoma ^ respond

@tachikoma ^ respond
tachikoma commented 2026-08-02 23:47:47 -04:00
Author
Collaborator
Copy Link

@andrew I checked the specific contract. The umask statement was correct: umask(2) declares mode_t umask(mode_t mask), returns the prior mask, and states that the call always succeeds. It has no -1/errno path.

uname(2) is different: it writes through struct utsname *buf and can return -1 with errno = EFAULT for an invalid buffer. I should not have implied that it was comparable to umask.

The correction to the proposed classification is that umask belongs with the mutating sys/stat.h surface, not the metadata-read wrappers in this ticket. Since it returns the previous mask rather than an error context, wrapping it needs a separate result-API decision; it should not be folded into this plan or treated as an ordinary errno wrapper.

@andrew I checked the specific contract. The `umask` statement was correct: [`umask(2)`](https://man7.org/linux/man-pages/man2/umask.2.html) declares `mode_t umask(mode_t mask)`, returns the prior mask, and states that the call always succeeds. It has no `-1`/`errno` path. [`uname(2)`](https://man7.org/linux/man-pages/man2/uname.2.html) is different: it writes through `struct utsname *buf` and can return `-1` with `errno = EFAULT` for an invalid buffer. I should not have implied that it was comparable to `umask`. The correction to the proposed classification is that `umask` belongs with the mutating `sys/stat.h` surface, not the metadata-read wrappers in this ticket. Since it returns the previous mask rather than an error context, wrapping it needs a separate result-API decision; it should not be folded into this plan or treated as an ordinary errno wrapper.
👍 1
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:medium 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#9
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?