Skip to content

__listds(): a failed record allocation drops one dataset and the scan continues #157

Description

@mgrossmann

__listds() loses datasets out of the middle of the list when a record
allocation fails, and the scan keeps running afterwards. The result is a list
that is short in a way no caller can see and no position can reveal.

This is the same defect family as #61 (__listvl()) and #80 defect 3
(__listpd()), but it fails one step worse than either of them, and it has more
live callers than the two of them together.

Filed 2026-08-30 while settling the convention those two ask for; the decision
recorded in #61 covers this function as well.

What happens

src/clib/@@listds.c:113-118, inside the parse() callback:

    /* allocate DSLIST record for dataset */
    dslist = calloc(1, sizeof(DSLIST));
    if (!dslist) {
        udata->dsn[0] = 0;
        udata->volser[0] = 0;
        goto quit;
    }

and the same shape for the array grow, :252-256:

done:
    /* add DSLIST record to array */
    rc = arrayadd(&udata->array, dslist);
    if (rc) {
        free(dslist);
        goto quit;
    }

quit:
    return 0;
}

quit: is followed by return 0. __listds() itself (:26-38) hands
__listc() the callback and then returns udata.array unconditionally,
whatever happened inside.

Why this is worse than #61 and #80 defect 3

Those two truncate: the walk stops and the caller gets a short tail. This one
resumes. __listc() keeps feeding LISTCAT lines, parse() is called again
for the next dataset, and if storage has become available in the meantime the
entries after the failure are added normally.

So the array can be missing entries from the middle while still ending at the
last dataset the catalog returned. A caller sweeping the list for a name gets a
false negative on a list that looks complete — there is no short tail to
notice, no count to compare against, and the last element is exactly where it
would be on a good run.

errno is set by the allocator (calloc.c:21, @@aradd.c:28,49), but nothing
between there and the caller preserves it, and __listds() documents no
contract for it.

Who is affected

Every one of these is live and built:

Caller Use What a dropped entry does
mvsmf/src/dsapi.c:1313 the data set list endpoint a data set missing from a listing that looks complete
ftpd/src/ftpd#mvs.c:1032 LIST on a level a silently short listing
ftpd/src/ftpd#mvs.c:959 RECFM lookup for one named data set nothing — it falls back to a __locate() + __dscbdv() DSCB lookup
mvsmf/src/testapi.c:405 test endpoint short listing

mvsmf/src/dsapi.c:1313 is the case that matters. It is a multi-entry
listing a client renders and then picks from, so a dropped middle entry is the
false negative described above: the list ends where it should, nothing about it
looks short, and the data set is simply not in it.

ftpd#mvs.c:1032 is a listing too, so there it is a short display rather than a
wrong answer — and its empty path is the best-handled use of __listds() in
the ecosystem: :1040 takes a second opinion from __locate() before deciding
the prefix does not exist. Worth noting, because that is the shape the
convention below relies on. ftpd#mvs.c:959 cannot be misled at all; it wants
one data set's RECFM and already has a DSCB fallback for the empty case.

The complication: the callback cannot stop the scan

The obvious fix — have parse() return non-zero and stop — is not available
today. __listc() discards the callback's return value:

src/clib/@@listc.c:67    prt(udata, "%s", p);
src/clib/@@listc.c:93    prt(udata, "%s", p);

and cliblist.h:280-291 documents __listc()'s own return (the IDCAMS return
code) while saying nothing about what prt() returning something is supposed to
mean. So either:

  1. a flag in UDATA that parse() sets on failure and tests at entry, so
    every later line is skipped — no signature change anywhere, entirely inside
    @@listds.c; or
  2. make __listc() honour a non-zero prt() return and stop the scan. That
    is a documented contract for a public function with other callers, so it
    wants its own decision.

(1) is enough for this issue. (2) may be worth having anyway, but not as a
passenger here.

What the fix has to do

Per the convention settled in #61 for all four list builders:

  • free the partial array and return NULL,
  • with errno guaranteed to be ENOMEM at the return,
  • and errno = 0 on entry, so a caller that reads it after a NULL from an
    empty catalog level does not see a stale value.

Note the errno survival question is real here and needs measuring rather than
assuming: the failure happens inside parse(), and everything __listc() does
afterwards — the remaining reads, fclose(), the temp data set teardown — runs
between the failure and the caller. Saving the value into UDATA at the point
of failure and restoring it in __listds() before return is the form that
does not depend on that.

Test

No host test exists for __listds(). It goes through __listc(), which runs
IDCAMS into a temp data set, so a host test has to shim at the __listc()
boundary rather than the FILE one — a different harness from
test/host/tstlspd.c, which #includes the TU and shims fopen/fread.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions