Drop the historical narration from hts_lastcharptr's contract

The declaration is the API surface, so it keeps the double-evaluation
warning a caller cannot derive from the signature; what the macro
replaced is git's job. Its grep exclusion in the test goes with it,
since that comment was the only htssafe.h line the scan matched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

src/htsbauth.c

@@ -491,7 +491,7 @@ char *bauth_prefix(char *prefix, const char *adr, const char *fil) {
   if (a)
     *a = '\0';
   if (strchr(prefix, '/')) {
-    a = prefix + strlen(prefix) - 1;
+    a = hts_lastcharptr(prefix);
     while(*a != '/')
       a--;
     *(a + 1) = '\0';

src/htscore.c

@@ -3774,8 +3774,7 @@ int htsAddLink(htsmoduleStruct * str, char *link) {
         strcpybuff(codebase, heap(ptr)->fil);
       else
         strcpybuff(codebase, heap(heap(ptr)->precedent)->fil);
-      // empty codebase has no last char; codebase-1 would underflow
-      a = codebase[0] != '\0' ? codebase + strlen(codebase) - 1 : codebase;
+      a = hts_lastcharptr(codebase);
       while((*a) && (*a != '/') && (a > codebase))
         a--;
       if (*a == '/')

src/htslib.c

@@ -3309,8 +3309,9 @@ int ishtml(httrackp * opt, const char *fil) {
   }
 
   /* Search for known ext */
-  for(a = fil_noquery + strlen(fil_noquery) - 1;
-      *a != '.' && *a != '/' && a > fil_noquery; a--) ;
+  for (a = hts_lastcharptr(fil_noquery);
+       *a != '.' && *a != '/' && a > fil_noquery; a--)
+    ;
   if (*a == '.') {              // a une extension
     char BIGSTK fil_noquery[HTS_URLMAXSIZE * 2];
     char *b;
@@ -4247,9 +4248,8 @@ HTSEXT_API hts_boolean get_httptype_sized(httrackp *opt, char *s, size_t ssize,
     return 1;
   } else {
     /* Check html -> text/html */
-    const char *a = fil + strlen(fil) - 1;
+    const char *a = hts_lastcharptr(fil);
 
-    /* a < fil when fil is empty: bound before dereferencing */
     while ((a > fil) && (*a != '.') && (*a != '/'))
       a--;
     if (a >= fil && *a == '.' && strlen(a) < 32) {

src/htsname.c

@@ -823,7 +823,7 @@ int url_savename(lien_adrfilsave *const afs,
   // Change the extension? e.g. php3 saved as html, cgi as html or gif/xbm
   // depending on the resolved type.
   if (ext_chg && !opt->no_type_change) {
-    char *a = fil + strlen(fil) - 1;
+    char *a = hts_lastcharptr(fil);
 
     if ((opt->debug > 1) && (opt->log != NULL)) {
       if (ext_chg == 1)
@@ -858,7 +858,7 @@ int url_savename(lien_adrfilsave *const afs,
   }
   // Rechercher premier / et dernier .
   {
-    const char *a = fil + strlen(fil) - 1;
+    const char *a = hts_lastcharptr(fil);
 
     // passer structures
     start_pos = fil;
@@ -1203,29 +1203,29 @@ int url_savename(lien_adrfilsave *const afs,
     switch (opt->savename_type % 100) {
     case 4:
     case 5:{                   // séparer par types
-        const char *a = fil + strlen(fil) - 1;
+      const char *a = hts_lastcharptr(fil);
 
-        // passer structures
-        while((a > fil) && (*a != '/') && (*a != '\\'))
+      // passer structures
+      while ((a > fil) && (*a != '/') && (*a != '\\'))
+        a--;
+      if ((*a == '/') || (*a == '\\'))
+        a++;
+
+      // html?
+      if ((ext_chg != 0) ? (ishtml_ext(ext) == 1) : (ishtml(opt, fil) == 1)) {
+        if (opt->savename_type % 100 == 5)
+          strcatbuff(afs->save, "html/");
+      } else {
+        const char *a = hts_lastcharptr(fil);
+
+        while ((a > fil) && (*a != '/') && (*a != '.'))
           a--;
-        if ((*a == '/') || (*a == '\\'))
-          a++;
-
-        // html?
-        if ((ext_chg != 0) ? (ishtml_ext(ext) == 1) : (ishtml(opt, fil) == 1)) {
-          if (opt->savename_type % 100 == 5)
-            strcatbuff(afs->save, "html/");
-        } else {
-          const char *a = fil + strlen(fil) - 1;
-
-          while((a > fil) && (*a != '/') && (*a != '.'))
-            a--;
-          if (*a != '.')
-            strcatbuff(afs->save, "other");
-          else
-            strcatbuff(afs->save, a + 1);
-          strcatbuff(afs->save, "/");
-        }
+        if (*a != '.')
+          strcatbuff(afs->save, "other");
+        else
+          strcatbuff(afs->save, a + 1);
+        strcatbuff(afs->save, "/");
+      }
         /*strcatbuff(save,a); */
         /* add name */
         ADD_STANDARD_NAME(0);
@@ -1258,7 +1258,7 @@ int url_savename(lien_adrfilsave *const afs,
         }
         afs->save[i + j] = '\0';
         // ajouter extension
-        a = fil + strlen(fil) - 1;
+        a = hts_lastcharptr(fil);
         while((a > fil) && (*a != '/') && (*a != '.'))
           a--;
         if (*a == '.') {
@@ -1303,7 +1303,7 @@ int url_savename(lien_adrfilsave *const afs,
   // cela évite les /chez/toto et les /chez/toto/index.html incompatibles
   if (opt->savename_type != -1 &&
       opt->savename_delayed != HTS_SAVENAME_DELAYED_HARD) {
-    char *a = afs->save + strlen(afs->save) - 1;
+    char *a = hts_lastcharptr(afs->save);
 
     while((a > afs->save) && (*a != '.') && (*a != '/'))
       a--;
@@ -1420,8 +1420,10 @@ int url_savename(lien_adrfilsave *const afs,
   if (opt->savename_83 > 0) {
     char *a, *last;
 
-    for(last = afs->save + strlen(afs->save) - 1;
-        last != afs->save && *last != '/' && *last != '\\' && *last != '.'; last--) ;
+    for (last = hts_lastcharptr(afs->save);
+         last != afs->save && *last != '/' && *last != '\\' && *last != '.';
+         last--)
+      ;
     if (*last != '.') {
       last = NULL;
     }
@@ -1676,7 +1678,7 @@ int url_savename(lien_adrfilsave *const afs,
 #endif
         } else {                // utilisé par un AUTRE, changer de nom
           char BIGSTK tempo[HTS_URLMAXSIZE * 2];
-          char *a = afs->save + strlen(afs->save) - 1;
+          char *a = hts_lastcharptr(afs->save);
           size_t stem;
           int n = 2;
           char collisionSeparator =
@@ -1708,7 +1710,7 @@ int url_savename(lien_adrfilsave *const afs,
           // en plus il faut gérer le 8-3 .. pas facile le client
           if (opt->savename_83) {
             int max;
-            char *a = tempo + strlen(tempo) - 1;
+            char *a = hts_lastcharptr(tempo);
 
             while((a > tempo) && (*a != '/'))
               a--;

src/htsparse.c

@@ -2057,7 +2057,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                   }
                   q = strchr(a, '?');   // ne pas traiter après '?'
                   if (!q)
-                    q = a + strlen(a) - 1;
+                    q = hts_lastcharptr(a);
                   while((p = strstr(a, "//")) && (!done)) {     // remplacer // par /
                     if (p > q) {    // après le ? (toto.cgi?param=1//2.3)
                       done = 1; // stopper
@@ -2208,7 +2208,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                 // OUI!!
 #if HTS_TILDE_SLASH
                 if (hts_lastchar(lien) != '/') {
-                  char *a = lien + strlen(lien) - 1;
+                  char *a = hts_lastcharptr(lien);
 
                   // éviter aussi index~1.html
                   while(a > lien && (*a != '~') && (*a != '/')
@@ -2254,7 +2254,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                 // vérifier que l'on ne doit pas ajouter de .class
                 if (!error) {
                   if (add_class) {
-                    char *a = lien + strlen(lien) - 1;
+                    char *a = hts_lastcharptr(lien);
 
                     while((a > lien) && (*a != '/') && (*a != '.'))
                       a--;
@@ -2309,7 +2309,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                         {
                           char *a;
 
-                          a = lien + strlen(lien) - 1;
+                          a = hts_lastcharptr(lien);
                           while((*a) && (*a != '/') && (a > lien))
                             a--;
                           if (*a == '/') {
@@ -2320,8 +2320,8 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                     }
 
                     if (!error) {       // pas d'erreur?
-                      if (p_type == 2) {        // code ET PAS codebase      
-                        char *a = lien + strlen(lien) - 1;
+                      if (p_type == 2) { // code ET PAS codebase
+                        char *a = hts_lastcharptr(lien);
                         char *start_of_filename = jump_identification(lien);
 
                         if (start_of_filename != NULL

src/htssafe.h

@@ -590,6 +590,17 @@ static HTS_INLINE HTS_UNUSED void hts_rtrim(char *s, const char *set) {
   s[hts_rtrimlen(s, set)] = '\0';
 }
 
+/* Offset of the last character of s, or 0 when s is empty. */
+static HTS_INLINE HTS_UNUSED size_t hts_lastcharoffset(const char *s) {
+  const size_t len = strlen(s);
+
+  return len != 0 ? len - 1 : 0;
+}
+
+/* Address of the last character of S, or of its terminating NUL when S is
+   empty. S is evaluated twice, so pass an lvalue. */
+#define hts_lastcharptr(S) ((S) + hts_lastcharoffset(S))
+
 /* Thin aliases over the libc allocator/memcpy (historical "t" suffix); no
    added bounds checking. freet() also NULLs the freed pointer and tolerates
    NULL. memcpybuff() despite the name is a raw memcpy: the caller owns the

src/htsselftest.c

@@ -6710,9 +6710,10 @@ static int st_changes_race(httrackp *opt, int argc, char **argv) {
   return err;
 }
 
-/* The x[strlen(x) - 1] class (#770). The string starts mid-arena so the byte
-   it must not touch is a real neighbour; poisoned with '#', not 0, or a stray
-   NUL terminator would read as untouched. */
+/* The x[strlen(x) - 1] class (#770) and its pointer spelling x + strlen(x) - 1
+   (#781). The string starts mid-arena so the byte it must not touch is a real
+   neighbour; poisoned with '#', not 0, or a stray NUL terminator would read as
+   untouched. */
 static int st_lastchar(httrackp *opt, int argc, char **argv) {
   enum { off = 8 };
 
@@ -6779,11 +6780,52 @@ static int st_lastchar(httrackp *opt, int argc, char **argv) {
   CHECK(s[0] == '\0');
   CHECK(arena[guard] == '#');
 
+  /* the pointer spelling (#781): on an empty string the address must be the
+     terminating NUL, never the byte before it */
+  REPOISON("");
+  CHECK(hts_lastcharoffset(s) == 0);
+  CHECK(hts_lastcharptr(s) == s);
+  CHECK(*hts_lastcharptr(s) == '\0');
+  *hts_lastcharptr(s) = 'Z'; /* a write through it must stay inside s */
+  CHECK(arena[guard] == '#');
+  CHECK(s[0] == 'Z');
+
+  /* the neighbour must not be mistaken for the string's own last byte */
+  REPOISON("");
+  arena[guard] = '/';
+  CHECK(hts_lastcharptr(s) == s);
+  CHECK(*hts_lastcharptr(s) != '/');
+  CHECK(arena[guard] == '/');
+
+  /* the walk-back loops the sites use must stop at once on an empty string */
+  REPOISON("");
+  {
+    const char *p = hts_lastcharptr(s);
+    int steps = 0;
+
+    while (p > s && *p != '/')
+      p--, steps++;
+    CHECK(steps == 0);
+    CHECK(p == s);
+  }
+
+  REPOISON("ab/");
+  CHECK(hts_lastcharoffset(s) == 2);
+  CHECK(hts_lastcharptr(s) == s + 2);
+  CHECK(*hts_lastcharptr(s) == '/');
+  REPOISON("/");
+  CHECK(hts_lastcharptr(s) == s);
+  CHECK(*hts_lastcharptr(s) == '/');
+  CHECK(arena[guard] == '#');
+
   /* control: the canary must be able to fail, or the checks above prove
      nothing. Clobber it exactly as the unguarded idiom would. */
   REPOISON("");
   s[-1] = '\0';
   CHECK(arena[guard] != '#');
+  REPOISON("");
+  *(s + strlen(s) - 1) = 'X';
+  CHECK(arena[guard] != '#');
 
 #undef REPOISON
 #undef CHECK
@@ -6932,7 +6974,8 @@ static const struct selftest_entry {
     {"strsafe", "[overflow|overflow-buff|overflow-src [str]]",
      "bounded string-op self-test", st_strsafe},
     {"copyopt", "", "copy_htsopt option-copy self-test", st_copyopt},
-    {"lastchar", "", "last-char helpers never index before the buffer (#770)",
+    {"lastchar", "",
+     "last-char helpers never index before the buffer (#770, #781)",
      st_lastchar},
     {"rtrim", "", "hts_rtrim never walks below the buffer", st_rtrim},
     {"changes", "", "--changes bucket accounting and JSON escaping (#714)",

src/htstools.c

@@ -324,8 +324,7 @@ int lienrelatif(char *s, size_t ssize, const char *link, const char *curr_fil) {
   curr = _curr;
   strlcpybuff(curr, curr_fil, sizeof(_curr));
   if ((a = strchr(curr, '?')) == NULL) { // cut at the ? (query parameters)
-    // an empty path has no last character: curr-1 would read before the buffer
-    a = curr[0] != '\0' ? curr + strlen(curr) - 1 : curr;
+    a = hts_lastcharptr(curr);
   }
   while((*a != '/') && (a > curr))
     a--;                        // chercher dernier / du chemin courant

src/proxy/store.c

@@ -903,10 +903,11 @@ int PT_LoadCache__New(PT_Index index_, const char *filename) {
       coucal hashtable = index->hash;
 
       /* Compute base path for this index - the filename MUST be absolute! */
-      for(slashes = 2, abpath = filename + (int) strlen(filename) - 1;
-          abpath > filename && ((*abpath != '/' && *abpath != '\\')
-                                || --slashes > 0);
-          abpath--) ;
+      for (slashes = 2, abpath = hts_lastcharptr(filename);
+           abpath > filename &&
+           ((*abpath != '/' && *abpath != '\\') || --slashes > 0);
+           abpath--)
+        ;
       index->path[0] = '\0';
       if (slashes == 0 && *abpath != 0) {
         int i;
@@ -1499,10 +1500,11 @@ static int PT_LoadCache__Old(PT_Index index_, const char *filename) {
 
       /* -------------------- COPY OF THE __New() CODE -------------------- */
       /* Compute base path for this index - the filename MUST be absolute! */
-      for(slashes = 2, abpath = filename + (int) strlen(filename) - 1;
-          abpath > filename && ((*abpath != '/' && *abpath != '\\')
-                                || --slashes > 0);
-          abpath--) ;
+      for (slashes = 2, abpath = hts_lastcharptr(filename);
+           abpath > filename &&
+           ((*abpath != '/' && *abpath != '\\') || --slashes > 0);
+           abpath--)
+        ;
       index->path[0] = '\0';
       if (slashes == 0 && *abpath != 0) {
         int i;

tests/01_engine-lastchar.test

@@ -1,8 +1,8 @@
 #!/bin/bash
 #
-# Runs the lastchar self-test (#770), then keeps the idiom it replaced from
-# coming back: x[strlen(x) - 1] indexes one byte before the buffer when x is
-# empty, and the guard is never where the index is.
+# Runs the lastchar self-test (#770, #781), then keeps the idiom it replaced
+# from coming back: x[strlen(x) - 1] and x + strlen(x) - 1 both land one byte
+# before the buffer when x is empty, and the guard is never where they are.
 
 set -eu
 
@@ -33,3 +33,36 @@ test -z "$hits" || {
     echo "$hits" >&2
     exit 1
 }
+
+# Same for the pointer spelling. The three survivors sit in commented-out
+# blocks in htsname.c; the rest are this class being named, plus the self-test's
+# own deliberate underflow.
+hits=$(command grep -rnE '\+ *\(?(int|size_t)?\)? *strlen *\([^)]*\) *- *1' \
+    "$top_srcdir/src" --include='*.c' --include='*.h' --exclude-dir=coucal |
+    grep -v 'htsname\.c:.*char\* a=\(fil+strlen(fil)\|save+strlen(save)\)-1;' |
+    grep -v 'htsselftest\.c:.*pointer spelling x + strlen(x) - 1' |
+    grep -v 'htsselftest\.c:.*\*(s + strlen(s) - 1) =' || true)
+
+test -z "$hits" || {
+    echo "x + strlen(x) - 1 reintroduced; use hts_lastcharptr:" >&2
+    echo "$hits" >&2
+    exit 1
+}
+
+# url_savename() has nine of these sites and reaches all of them with an empty
+# "fil", which "?x=1" produces. Names are asserted loosely, since the point is
+# the run: on unfixed code the sanitized CI leg aborts inside htsname.c.
+for args in "?x=1 text/plain" "?x=1 text/html" "?x=1 text/html type=4"; do
+    # shellcheck disable=SC2086
+    name=$(httrack -#test=savename $args) || {
+        echo "httrack -#test=savename $args exited non-zero: $name" >&2
+        exit 1
+    }
+    case "$name" in
+    "savename: "?*) ;;
+    *)
+        echo "-#test=savename $args produced no name: $name" >&2
+        exit 1
+        ;;
+    esac
+done