0.6.1 --- the same bounded-transfer defect, and the tests that had never run

.github/workflows/ci.yml

@@ -215,6 +215,32 @@ jobs:
           }
           subst mcpp.toml .spec
 
+      # THIS PACKAGE'S OWN TESTS HAD NEVER RUN.
+      #
+      # tests/ has held five suites since this implementation was written and no
+      # workflow invoked them. What ran was the specification's suite, which is a
+      # different instrument: it examines what every implementation must do, and
+      # these examine what this one does with the system beneath it. A defect
+      # visible only to the second kind was therefore invisible here, which is
+      # what happened to openkal.timeout -- there was no observation of it in
+      # either place that a wait upon the wrong descriptor did not satisfy.
+      #
+      # The step is openkal-linux's, unchanged, including the assertion that
+      # every suite ran: a suite that discovered nothing reports success.
+      - name: This package's own tests
+        run: |
+          set -euo pipefail
+          mcpp test 2>&1 | tee tests.log
+          # The list is derived from the files present rather than written out
+          # here: a hand-written list names the suites that existed when it was
+          # written, and one added afterwards escapes the assertion silently.
+          missing=0
+          for f in tests/*.cpp; do
+            name="$(basename "$f" .cpp)"
+            grep -q "^$name \.\.\. ok" tests.log || { echo "did not run or did not pass: $name" >&2; missing=1; }
+          done
+          test "$missing" -eq 0
+
       # The other architecture, as far as this system allows it to be reached.
       #
       # The system-call numbers agree between the two --- measured, in the

README.md

@@ -8,7 +8,7 @@ written on the kernel's own calls.
 openkal = "0.9.0"
 
 [target.'cfg(os = "macos")'.dependencies]
-openkal-macos = "0.6.0"
+openkal-macos = "0.6.1"
 ```
 
 Its purpose is as much to test the specification as to be used. A specification

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "openkal-macos"
-version     = "0.6.0"
+version     = "0.6.1"
 description = "An implementation of openkal for macOS, written on the kernel's own calls. Its purpose is as much to test the specification as to be used."
 license     = "Apache-2.0"
 

src/handle.h

@@ -24,6 +24,18 @@ inline okm_uptr pack(int fd) {
 }
 
 // Returns the descriptor, or -1 if the word does not name a live one.
+//
+// THIS ACCEPTS A WORD THAT WAS NEVER PACKED, AND SILENTLY. A bare descriptor N
+// has the shape of a packed handle naming N-1 whose generation is still zero,
+// so this returns N-1 for it rather than -1. Nothing here can tell the two
+// apart: the word is one machine word and carries no tag.
+//
+// The consequence is that a handle of the OTHER discipline must never reach
+// this function. openkal.stream's handles are bare descriptors (stream.cpp
+// states why), and src/timeout.cpp used to pass one here and wait upon the
+// descriptor below the one it then transferred upon. Owned handles --- kal_file,
+// kal_dir, kal_net_listener, kal_net_conn, kal_datagram --- are the whole of
+// this function's domain.
 inline int unpack(okm_uptr h) {
     const int fd = static_cast<int>(h & 0xffffffffu) - 1;
     if (fd < 0 || fd >= kMaxDescriptor) return -1;

src/timeout.cpp

@@ -59,6 +59,36 @@ int await(int fd, short events, kal_u64 ns) {
     return kal_ok;
 }
 
+// A STREAM HANDLE IS A DESCRIPTOR AND IS NOT DECODED. stream.cpp states it in
+// terms, kal_stream_read and kal_stream_write take it as one, and so must the
+// wait that precedes them: the wait and the transfer that follows it have to
+// name the same object or the wait answers about something else.
+//
+// THIS FILE USED TO DECODE IT AS AN OWNED HANDLE, AND THE DECODE SUCCEEDED.
+// handle.h packs an owned handle as (generation << 32) | (fd + 1), so a bare
+// descriptor N has exactly the shape of a packed handle naming N-1, and
+// okm::unpack accepts it whenever generation N-1 is still zero -- which is
+// every index at which no owned handle has yet been released. The wait was
+// therefore performed upon descriptor N-1 and the transfer upon N.
+//
+// The former reading tested `unpack` and fell back when it failed, on the
+// stated ground that the standard streams are not packed. That ground is
+// correct and the conclusion drawn from it was not: NO stream handle is packed
+// here, and of the three standard ones only kal_stdin, whose handle is zero,
+// fails to decode. kal_stdout decoded to descriptor 0 and kal_stderr to 1.
+//
+// What a caller observed was an expiry for a stream that had bytes waiting, or
+// a wait without bound inside an operation that states one, according to what
+// happened to occupy the descriptor below. Both are functions of the process's
+// descriptor history, so the same program answered differently when run alone
+// and when run after something else -- and the index healed for good once an
+// owned handle at N-1 had been released, because that advances the generation
+// and makes the decode fail correctly.
+//
+// Found in openkal-linux, whose timeout.cpp is this file's counterpart and
+// carried the same four call sites with the same two wrong.
+int stream_fd(kal_stream s) { return static_cast<int>(s.h); }
+
 }  // namespace
 
 extern "C" {
@@ -68,28 +98,28 @@ kal_intptr kal_timeout_read(kal_stream s, void* buf, kal_uintptr len, kal_u64 ns
     // would turn a call that always succeeds into one that can expire.
     if (len == 0) return 0;
 
-    const int fd = okm::unpack(s.h);
-    // THE STANDARD STREAMS ARE NOT PACKED HANDLES. openkal.stream reports them
-    // as the descriptors themselves, so a word that does not unpack is taken to
-    // be one of those rather than being refused.
-    const int use = (fd >= 0) ? fd : static_cast<int>(s.h);
-
-    if (const int rc = await(use, static_cast<short>(okm::poll_in), ns); rc != kal_ok)
+    if (const int rc = await(stream_fd(s), static_cast<short>(okm::poll_in), ns);
+        rc != kal_ok)
         return -rc;
     return kal_stream_read(s, buf, len);
 }
 
 kal_intptr kal_timeout_write(kal_stream s, const void* buf, kal_uintptr len, kal_u64 ns) {
     if (len == 0) return 0;
 
-    const int fd  = okm::unpack(s.h);
-    const int use = (fd >= 0) ? fd : static_cast<int>(s.h);
-
-    if (const int rc = await(use, static_cast<short>(okm::poll_out), ns); rc != kal_ok)
+    if (const int rc = await(stream_fd(s), static_cast<short>(okm::poll_out), ns);
+        rc != kal_ok)
         return -rc;
     return kal_stream_write(s, buf, len);
 }
 
+// THE TWO OPERATIONS BELOW DO DECODE, AND THAT IS NOT AN INCONSISTENCY WITH THE
+// TWO ABOVE. A listener and a datagram are owned: their handles are made by
+// okm::pack and released by okm::retire, so the decode is the operation that
+// recovers the descriptor and its failure is how a released handle is refused.
+// A stream is borrowed and carries no generation. The four call sites divide
+// exactly along that line, and the two that were wrong were the two that had a
+// borrowed handle in hand.
 int kal_timeout_accept(kal_net_listener l, kal_u64 ns, kal_net_conn* out) {
     if (out == nullptr) return kal_err_invalid;
     const int fd = okm::unpack(l.h);

tests/conformance_env_time.cpp

@@ -46,11 +46,11 @@ int main() {
 
     // The wall source is claimed by this implementation, so it must report a
     // time after the specification was written rather than zero.
-    check(kal::time::has(kal::time::prop_wall_available), "the wall source is claimed");
+    check(kal::time::has(kal::time::wall_available), "the wall source is claimed");
     check(kal::time::wall() > 1700000000ull * 1000000000ull,
           "the wall source reports a plausible time");
 
-    const char ok[] = "openkal-linux: env and time conformance\n";
+    const char ok[] = "openkal-macos: env and time conformance\n";
     kal::write(kal::out(), ok, sizeof(ok) - 1);
     return failures == 0 ? 0 : 1;
 }

tests/conformance_fs.cpp

@@ -100,7 +100,7 @@ int main() {
     check(kal_fs_remove(root, "okl_dir", 7) == kal_ok, "the directory is removed");
     check(kal_fs_remove(root, "okl_probe.txt", 13) == kal_ok, "the file is removed");
 
-    const char ok[] = "openkal-linux: file system conformance\n";
+    const char ok[] = "openkal-macos: file system conformance\n";
     kal::write(kal::out(), ok, sizeof(ok) - 1);
     return failures == 0 ? 0 : 1;
 }

tests/conformance_memory.cpp

@@ -16,7 +16,7 @@ int main() {
     // cannot be used.
     if (kal::alloc(0, 8) != nullptr) return 1;
 
-    const char ok[] = "openkal-linux: memory conformance\n";
+    const char ok[] = "openkal-macos: memory conformance\n";
     kal::write(kal::out(), ok, sizeof(ok) - 1);
     return 0;
 }

tests/conformance_process_task.cpp

@@ -6,6 +6,20 @@ import openkal.stream;
 
 namespace {
 int failures = 0;
+
+void say(const char* s) {
+    kal_uintptr n = 0; while (s[n]) ++n;
+    kal::write(kal::err(), s, n);
+}
+
+void say_num(int v) {
+    if (v < 0) { kal::write(kal::err(), "-", 1); v = -v; }
+    char b[12]; int i = 12;
+    if (v == 0) b[--i] = '0';
+    while (v > 0) { b[--i] = static_cast<char>('0' + v % 10); v /= 10; }
+    kal::write(kal::err(), b + i, static_cast<kal_uintptr>(12 - i));
+}
+
 void check(bool ok, const char* what) {
     if (ok) return;
     ++failures;
@@ -57,17 +71,52 @@ int main() {
         const char* false_paths[] = { "bin/false", "usr/bin/false" };
         const kal_uintptr false_lens[] = { 9, 13 };
 
+        // THE CANDIDATE IS CHOSEN BY ASKING, AND IT USED TO BE CHOSEN BY
+        // SPAWNING AND RETRYING. That retry could not work and never ran:
+        // kal_process_spawn reports whether the DUPLICATE was made, and the
+        // program is replaced afterwards, inside a copy the caller no longer
+        // is. A path that does not exist therefore produces kal_ok and a
+        // duplicate that finishes with 127, so the first candidate was always
+        // taken and the second was unreachable code.
+        //
+        // On a system holding /usr/bin/true and no /bin/true the consequence
+        // was `status == 127' at the observation below, which is what this
+        // system reported the first time these suites were ever run. The
+        // observation after it -- that a non-zero status is reported as such --
+        // held throughout, upon a program that was never started.
+        auto locate = [&](const char* const* paths, const kal_uintptr* lens_) -> int {
+            for (int i = 0; i < 2; ++i) {
+                kal_node_info info{}; info.self_size = sizeof info;
+                if (kal_fs_info(slash, paths[i], lens_[i], 0, kal::fs::field::kind, &info) != kal_ok)
+                    continue;
+                if (info.kind != kal_node_absent) return i;
+            }
+            return -1;
+        };
+
+        const int t = locate(true_paths, true_lens);
+        const int fpath = locate(false_paths, false_lens);
+        check(t >= 0, "a program that succeeds is found");
+        check(fpath >= 0, "a program that fails is found");
+
         kal_process p{};
         const char* argv[] = { "openkal" };
         const kal_uintptr lens[] = { 7 };
         int rc = kal_err_invalid;
-        for (int i = 0; i < 2 && rc != kal_ok; ++i)
-            rc = kal_process_spawn(slash, true_paths[i], true_lens[i], argv, lens, 1,
+        if (t >= 0)
+            rc = kal_process_spawn(slash, true_paths[t], true_lens[t], argv, lens, 1,
                                    nullptr, nullptr, 0, nullptr, &p);
         check(rc == kal_ok, "a program is started");
         if (rc == kal_ok) {
             int status = -1, terminated = -1;
             check(kal_process_wait(p, &status, &terminated) == kal_ok, "the program is waited for");
+            // The observed status is reported when it is wrong. `127' names an
+            // image that was not replaced and `0' names one that ran; without
+            // the number the two arrive as the same line.
+            if (!(status == 0 && terminated == 0)) {
+                say("  status="); say_num(status);
+                say(" terminated="); say_num(terminated); say("\n");
+            }
             check(status == 0 && terminated == 0, "the status it finished with is reported");
             kal_process_close(p);
         }
@@ -78,13 +127,19 @@ int main() {
         kal_process q{};
         const char* qargv[] = { "openkal" };
         int qrc = kal_err_invalid;
-        for (int i = 0; i < 2 && qrc != kal_ok; ++i)
-            qrc = kal_process_spawn(slash, false_paths[i], false_lens[i], qargv, lens, 1,
+        if (fpath >= 0)
+            qrc = kal_process_spawn(slash, false_paths[fpath], false_lens[fpath], qargv, lens, 1,
                                     nullptr, nullptr, 0, nullptr, &q);
+        check(qrc == kal_ok, "the program that fails is started");
         if (qrc == kal_ok) {
             int status = -1, terminated = -1;
             kal_process_wait(q, &status, &terminated);
-            check(status != 0, "a non-zero status is reported as such");
+            // NOT MERELY NON-ZERO. 127 is what a duplicate reports when the
+            // image was never replaced, so `!= 0' is satisfied by a program
+            // that did not run -- which is precisely how this observation held
+            // while the one above did not.
+            check(status == 1 && terminated == 0,
+                  "the status of a program that fails is its own and not 127");
             kal_process_close(q);
         }
 
@@ -105,12 +160,18 @@ int main() {
         const char* script = "test \"$0\" = openkal-observed-argv0";
         kal_uintptr script_len = 0; while (script[script_len]) ++script_len;
 
+        // Located rather than retried, for the reason recorded above: the retry
+        // this replaces could not distinguish a path that does not exist from
+        // one that does, so it always took the first.
+        const int sh = locate(sh_paths, sh_lens);
+        check(sh >= 0, "a shell is found");
+
         kal_process r{};
         const char*       rargv[] = { "openkal-observed-argv0", "-c", script };
         const kal_uintptr rlens[] = { 22, 2, script_len };
         int rrc = kal_err_invalid;
-        for (int i = 0; i < 2 && rrc != kal_ok; ++i)
-            rrc = kal_process_spawn(slash, sh_paths[i], sh_lens[i], rargv, rlens, 3,
+        if (sh >= 0)
+            rrc = kal_process_spawn(slash, sh_paths[sh], sh_lens[sh], rargv, rlens, 3,
                                     nullptr, nullptr, 0, nullptr, &r);
         check(rrc == kal_ok, "a shell is started");
         if (rrc == kal_ok) {
@@ -153,7 +214,7 @@ int main() {
     kal_task_yield();
     check(kal_task_current() != 0, "the calling context has an identity");
 
-    const char ok[] = "openkal-linux: process and task conformance\n";
+    const char ok[] = "openkal-macos: process and task conformance\n";
     kal::write(kal::out(), ok, sizeof(ok) - 1);
     return failures == 0 ? 0 : 1;
 }

tests/conformance_stream.cpp

@@ -32,7 +32,7 @@ int main() {
     // A write transfers the whole buffer or reports why it could not. The
     // specification excludes a successful partial transfer, so a conforming
     // result reports either the full count or a non-zero error.
-    const char msg[] = "openkal-linux: conformance\n";
+    const char msg[] = "openkal-macos: conformance\n";
     // ⭐ ONE SIGNED WORD: the count, or the negated condition when no byte
     // moved. A caller never inspects two things to learn one thing.
     const kal_intptr r = kal::write(kal::out(), msg, sizeof(msg) - 1);