review: comment conciseness pass

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

src/htsback.c

@@ -1362,9 +1362,39 @@ int back_flush_output(httrackp * opt, cache_back * cache, struct_back * sback,
   return 0;
 }
 
+/* Move a still-writing .delayed placeholder to its final name (#483). */
+hts_boolean back_delayed_rename(httrackp *opt, lien_back *back,
+                                const char *newname) {
+  hts_boolean renamed;
+
+  if (!back->r.is_write || back->tmpfile != NULL ||
+      !IS_DELAYED_EXT(back->url_sav) || strcmp(back->url_sav, newname) == 0)
+    return HTS_TRUE; /* nothing bound to the placeholder name */
+  if (back->r.out != NULL) {
+    fclose(back->r.out);
+    back->r.out = NULL;
+  }
+  renamed = RENAME(back->url_sav, newname) == 0 ? HTS_TRUE : HTS_FALSE;
+  if (renamed && (back->status == STATUS_READY ||
+                  (back->r.out = FOPEN(newname, "ab")) != NULL)) {
+    filenote(&opt->state.strc, newname, NULL);
+    hts_log_print(opt, LOG_DEBUG, "moved placeholder %s to %s", back->url_sav,
+                  newname);
+    return HTS_TRUE;
+  }
+  /* partial lost: drop only what we own (Windows rename won't overwrite) */
+  hts_log_print(opt, LOG_WARNING | LOG_ERRNO, "unable to move %s to %s",
+                back->url_sav, newname);
+  back->r.statuscode = STATUSCODE_INVALID;
+  strcpybuff(back->r.msg, "Write error on disk");
+  back->r.is_write = 0;
+  (void) UNLINK(renamed ? newname : back->url_sav);
+  return HTS_FALSE;
+}
+
 // effacer entrée
 /* Discard a cancelled mid-write .delayed placeholder (unusable across runs). */
-static void back_delayed_discard(httrackp *opt, lien_back *back) {
+void back_delayed_discard(httrackp *opt, lien_back *back) {
   if (back->r.out != NULL) {
     fclose(back->r.out);
     back->r.out = NULL;

src/htsback.h

@@ -113,6 +113,12 @@ void back_set_locked(struct_back * sback, const int p);
 void back_set_unlocked(struct_back * sback, const int p);
 int back_delete(httrackp * opt, cache_back * cache, struct_back * sback,
                 const int p);
+/* Discard back's on-disk .delayed placeholder and its refname. */
+void back_delayed_discard(httrackp *opt, lien_back *back);
+/* Move back's .delayed placeholder (and open stream) to newname;
+   HTS_FALSE = file lost, slot flagged in error. */
+hts_boolean back_delayed_rename(httrackp *opt, lien_back *back,
+                                const char *newname);
 void back_index_unlock(struct_back * sback, const int p);
 int back_clear_entry(lien_back * back);
 int back_flush_output(httrackp * opt, cache_back * cache, struct_back * sback,

src/htsparse.c

@@ -4566,6 +4566,12 @@ int hts_wait_delayed(htsmoduleStruct * str, lien_adrfilsave *afs,
           back_maydelete(opt, cache, sback, b);    // cancel
           b = -1;
 
+          /* the cancel may leave the now-unreferenced placeholder on disk
+           * (#483) */
+          if (fexist_utf8(delayed_back.url_sav)) {
+            back_delayed_discard(opt, &delayed_back);
+          }
+
           /* Recompute filename with MIME type */
           afs->save[0] = '\0';
           url_savename(afs, former, heap(ptr)->adr,
@@ -4782,6 +4788,9 @@ int hts_wait_delayed(htsmoduleStruct * str, lien_adrfilsave *afs,
 
           /* Still have a back reference */
           if (b >= 0) {
+            /* move a still-writing placeholder before the url_sav patch
+               blinds every cleanup to it (#483) */
+            back_delayed_rename(opt, &back[b], afs->save);
             /* patch url_sav BEFORE finalize: it records/caches under this name
              */
             strcpybuff(back[b].url_sav, afs->save);

tests/34_local-maxtime.test

@@ -7,10 +7,8 @@ set -euo pipefail
 
 : "${top_srcdir:=..}"
 
-# cancelled crawls can orphan .delayed placeholders (#483): skip that audit
 start=$(date +%s)
 bash "$top_srcdir/tests/local-crawl.sh" \
-    --skip-delayed-audit \
     --log-found 'More than 2 seconds passed' \
     httrack 'BASEURL/trickle/index.html' -E2 -c4
 wall=$(($(date +%s) - start))

tests/39_local-delayed-cancel.test

@@ -0,0 +1,13 @@
+#!/bin/bash
+#
+# Cancelled delayed-type-checks must not orphan .delayed placeholders (#483).
+# Timing-dependent (hence two tries); -A keeps the window reachable.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+for _ in 1 2; do
+    bash "$top_srcdir/tests/local-crawl.sh" \
+        httrack 'BASEURL/dcancel/index.html' -E1 -c4 -A25000
+done

tests/Makefile.am

@@ -103,6 +103,7 @@ TESTS = \
 	35_local-maxsize.test \
 	36_local-bigcrawl.test \
 	37_local-cache-outage.test \
-	38_local-update-304.test
+	38_local-update-304.test \
+	39_local-delayed-cancel.test
 
 CLEANFILES = check-network_sh.cache

tests/local-crawl.sh

@@ -97,7 +97,6 @@ tmpdir=$(mktemp -d "${tmptopdir}/httrack_local.XXXXXX") || die "could not create
 # --- parse leading control flags --------------------------------------------
 declare -a audit=()
 declare -a cookies=()
-skip_delayed_audit=""
 scheme=http
 pos=0
 args=("$@")
@@ -123,9 +122,6 @@ while test "$pos" -lt "$nargs"; do
         pos=$((pos + 1))
         cookies+=("${args[$pos]}")
         ;;
-    --skip-delayed-audit)
-        skip_delayed_audit=1
-        ;;
     --errors | --files)
         audit+=("${args[$pos]}" "${args[$((pos + 1))]}")
         pos=$((pos + 1))
@@ -293,15 +289,12 @@ done
 test -n "$hostroot" || die "could not find host root under $out"
 debug "host root: $hostroot"
 
-# A completed crawl must leave no .delayed temporaries (issue #107).
-# --skip-delayed-audit: a cancelled crawl can orphan placeholders (issue #483)
-if test -z "$skip_delayed_audit"; then
-    info "checking for leftover .delayed files"
-    leftovers=$(find "$out" -name '*.delayed' 2>/dev/null | head -5)
-    if test -z "$leftovers"; then result "OK"; else
-        result "leftover: $leftovers"
-        exit 1
-    fi
+# No crawl, even a cancelled one, may leave .delayed temporaries (#107, #483).
+info "checking for leftover .delayed files"
+leftovers=$(find "$out" -name '*.delayed' 2>/dev/null | head -5)
+if test -z "$leftovers"; then result "OK"; else
+    result "leftover: $leftovers"
+    exit 1
 fi
 
 # --- audit -------------------------------------------------------------------

tests/local-server.py

@@ -913,6 +913,26 @@ class Handler(SimpleHTTPRequestHandler):
         except OSError:
             pass
 
+    # #483: trickled .bin pages so the -E stop lands in the type waiter's
+    # unlock-to-patch window with body bytes pending.
+    def route_dcancel_index(self):
+        self.send_bin_index()
+
+    def route_dcancel_page(self):
+        self.send_response(200)
+        self.send_header("Content-Type", "application/octet-stream")
+        self.send_header("Content-Length", "4096")
+        self.end_headers()
+        if self.command == "HEAD":
+            return
+        try:
+            for _ in range(32):
+                self.wfile.write(b"z" * 128)
+                self.wfile.flush()
+                time.sleep(0.05)
+        except OSError:
+            pass
+
     # -M byte cap (#77): large fast files so a crawl overruns -M immediately.
     BIGFILE_BYTES = 640 * 1024
 
@@ -976,6 +996,15 @@ class Handler(SimpleHTTPRequestHandler):
         "/trickle/p5.bin": route_trickle_page,
         "/trickle/p6.bin": route_trickle_page,
         "/trickle/p7.bin": route_trickle_page,
+        "/dcancel/index.html": route_dcancel_index,
+        "/dcancel/p0.bin": route_dcancel_page,
+        "/dcancel/p1.bin": route_dcancel_page,
+        "/dcancel/p2.bin": route_dcancel_page,
+        "/dcancel/p3.bin": route_dcancel_page,
+        "/dcancel/p4.bin": route_dcancel_page,
+        "/dcancel/p5.bin": route_dcancel_page,
+        "/dcancel/p6.bin": route_dcancel_page,
+        "/dcancel/p7.bin": route_dcancel_page,
         "/bigfiles/index.html": route_bigfiles_index,
         "/bigfiles/p0.bin": route_bigfile,
         "/bigfiles/p1.bin": route_bigfile,