bdc473369e1bb2a5666bd76626d449af50b81d40
braney
  Fri Jul 31 10:50:24 2026 -0700
resolve char * constants per file in the harvesters, not pooled refs #37925 refs #37923

Both harvesters merged file-scope "char *NAME = \"literal\";" definitions into
the same table as the #defines, keyed by identifier with setdefault, so one
program's private constant answered for every other file in the tree and
whichever file os.walk reached first won.  harvestCartVars.py already did this
per file; these two hoisted it and broke.

What it was getting wrong:

- hgConfCatalog listed encpipeline_prod as an hg.conf variable, verified,
cited at hgTracks.c:9867.  That line is cfgNamesWithPrefix(database), the
<db>_TopLink family, and encpipeline_prod is a hardcoded MySQL database
name in hg/encode/docId/docIdView/docIdView.c.  The row is gone and the
call site now reads {database}, a runtime value.
- Twelve URL records named the wrong variable.  Five cartRemove(cart,
varName) sites were reported as removing dnaLines, which is what an
assembly tool at primeMate.c:296 calls its own varName; three
cartRemove(cart, var) sites read as num.  They now read {varName} and
{var}.
- cfgValsWithPrefix passing its own prefix parameter through resolved to an
unrelated utility's "char *prefix = \"\";", and an empty string is falsy,
so the call site was dropped instead of reported.  It goes through
NAME_SKIP now like the other accessor plumbing.

The deliberate cost is the genuine cross-file constant: snp125ColorSourceOldVar
is defined in hg/cgilib/snp125Ui.c and declared extern in hg/inc/snp125Ui.h, so
its hgTrackUi call site now reads {snp125ColorSourceOldVar} rather than
snp125ColorSource.  Pooling only extern-declared names would recover it and
bring the collisions straight back, since database is extern in hgTracks and
separately initialized to a literal in that ENCODE tool.

hgConfCatalog --check still reports 0 problems and --reconcile is unchanged;
urlCommandCatalog --check still reports catalog ok with the persistence audit
at 41 known leaks, 0 unrecorded, 0 stale.

diff --git src/hg/utils/urlCommandCatalog/harvestUrlCommands.py src/hg/utils/urlCommandCatalog/harvestUrlCommands.py
index dbaa6fd0747..03c1b54160b 100755
--- src/hg/utils/urlCommandCatalog/harvestUrlCommands.py
+++ src/hg/utils/urlCommandCatalog/harvestUrlCommands.py
@@ -87,88 +87,105 @@
     rounds until nothing new appears.
     """
     macro = {}
     chains = []
     # #define NAME "literal"
     lit_re = re.compile(
         r'^\s*#\s*define\s+([A-Za-z_]\w*)\s+'
         r'("(?:[^"\\]|\\.)*")\s*(?:/[/*].*)?$')
     # #define NAME OTHER, or NAME OTHER "suffix", or NAME "prefix" OTHER
     cat_re = re.compile(
         r'^\s*#\s*define\s+([A-Za-z_]\w*)\s+'
         r'((?:(?:"(?:[^"\\]|\\.)*")|(?:[A-Za-z_]\w*))'
         r'(?:\s+(?:(?:"(?:[^"\\]|\\.)*")|(?:[A-Za-z_]\w*)))*)'
         r'\s*(?:/[/*].*)?$')
     piece_re = re.compile(r'"(?:[^"\\]|\\.)*"|[A-Za-z_]\w*')
-    # static char *dbCgiName = "db";  Not every name is a #define: web.c holds
-    # db, org and clade this way, and they are the most-used URL params there are.
-    const_re = re.compile(
-        r'^\s*(?:static\s+)?(?:const\s+)?char\s*\*\s*([A-Za-z_]\w*)\s*=\s*'
-        r'("(?:[^"\\]|\\.)*")\s*;')
 
     for fn in macro_files():
         for line in open(fn, errors="replace"):
             m = lit_re.match(line)
-            if m:
-                macro.setdefault(m.group(1), m.group(2)[1:-1])
-                continue
-            m = const_re.match(line)
             if m:
                 macro.setdefault(m.group(1), m.group(2)[1:-1])
                 continue
             m = cat_re.match(line)
             if m:
                 chains.append((m.group(1), piece_re.findall(m.group(2))))
 
     for _ in range(5):
         for name, pieces in chains:
             if name in macro:
                 continue
             out = ""
             for piece in pieces:
                 if piece.startswith('"'):
                     out += piece[1:-1]
                 elif piece in macro:
                     out += macro[piece]
                 else:
                     out = None
                     break
             if out is not None:
                 macro[name] = out
     return macro
 
 
-def resolve(tok, macro):
-    """Turn one C token into the name it stands for, or {ident} if unknown."""
+def resolve(tok, macro, localconst=None):
+    """Turn one C token into the name it stands for, or {ident} if unknown.
+
+    localconst is the file's own char * constants, which take precedence over
+    the shared #define table and must never be shared between files: see
+    CONST_RE for why.
+    """
     tok = tok.strip()
     if not tok:
         return None
     if tok.startswith('"') and tok.endswith('"') and len(tok) >= 2:
         return tok[1:-1]
     if tok == "NULL":
         return None
+    if localconst and tok in localconst:
+        return localconst[tok]
     if tok in macro:
         return macro[tok]
     if re.match(r'^[A-Za-z_]\w*$', tok):
         return "{%s}" % tok
     return None
 
 
 # ---------------------------------------------------------------------------
 # scanning
 # ---------------------------------------------------------------------------
 
+# static char *dbCgiName = "db";  Not every name is a #define: web.c holds db,
+# org and clade this way, and they are the most-used URL params there are.
+# Resolved per file and never pooled, because the same identifier means
+# different things in different programs, and a pooled table let whichever file
+# was walked first answer for all of them: five unrelated cartRemove(cart,
+# varName) sites were reported as removing "dnaLines", the value an assembly
+# tool happens to give its own varName.
+#
+# The cost is the genuine cross-file case, a const defined in one .c and
+# declared extern in a header: snp125ColorSourceOldVar (hg/cgilib/snp125Ui.c)
+# now reads as {snp125ColorSourceOldVar} at its hgTrackUi call site.  Pooling
+# only extern-declared names would recover it but bring the collisions back,
+# since `database` is extern in hgTracks and separately initialized to a
+# literal in an unrelated ENCODE tool.  One honest {ident} beats eleven
+# confident wrong answers.
+CONST_RE = re.compile(
+    r'^[ \t]*(?:static[ \t]+)?(?:const[ \t]+)?char[ \t]*\*[ \t]*([A-Za-z_]\w*)'
+    r'[ \t]*=[ \t]*("(?:[^"\\]|\\.)*")[ \t]*;', re.M)
+
 EXCLUDE_RE = re.compile(r'\bchar\s*\*\s*excludeVars\s*\[\s*\]\s*=\s*\{')
 
 # cgiOptionalString / cgiUsualString / cgiVarExists / cgiOptionalInt / cgiString
 # / cgiBoolean / cgiBooleanDefined / cgiUsualInt ...
 CGI_READ_RE = re.compile(
     r'\bcgi(?:Optional|Usual)?'
     r'(?:String|Int|Double|Boolean|BooleanDefined|VarExists)?'
     r'\s*\(\s*("(?:[^"\\]|\\.)*"|[A-Za-z_]\w*)\s*[,)]')
 
 CART_REMOVE_RE = re.compile(
     r'\bcartRemove(?:Prefix|Like)?\s*\(\s*\w+\s*,\s*'
     r'("(?:[^"\\]|\\.)*"|[A-Za-z_]\w*)\s*[,)]')
 
 # Reads that are not URL commands: these fetch the value of a name held in a
 # variable, or are the cart's own plumbing.
@@ -205,99 +222,102 @@
 
 
 def rel(path):
     return os.path.relpath(path, ROOT)
 
 
 def owner(path):
     """Which CGI or library a file belongs to, for grouping."""
     r = rel(path)
     parts = r.split(os.sep)
     if len(parts) >= 2:
         return os.sep.join(parts[:-1])
     return r
 
 
-def find_exclude_vars(text, path, macro):
+def find_exclude_vars(text, path, macro, localconst):
     """Pull the members out of every excludeVars[] declaration in one file."""
     out = []
     for m in EXCLUDE_RE.finditer(text):
         start = m.end()
         depth = 1
         i = start
         instr = False
         while i < len(text) and depth:
             c = text[i]
             if instr:
                 if c == "\\":
                     i += 2
                     continue
                 if c == '"':
                     instr = False
             elif c == '"':
                 instr = True
             elif c == "{":
                 depth += 1
             elif c == "}":
                 depth -= 1
             i += 1
         body = text[start:i-1]
         line = text.count("\n", 0, m.start()) + 1
         # strip comments so a commented-out member is not harvested
         body = re.sub(r'/\*.*?\*/', '', body, flags=re.S)
         body = re.sub(r'//[^\n]*', '', body)
         for tok in body.split(","):
-            name = resolve(tok, macro)
+            name = resolve(tok, macro, localconst)
             if name:
                 out.append((name, "%s:%d" % (rel(path), line)))
     return out
 
 
-def find_matches(regex, text, path, macro, skip=()):
+def find_matches(regex, text, path, macro, localconst, skip=()):
     out = []
     for m in regex.finditer(text):
         tok = m.group(1)
         if tok in skip:
             continue
-        name = resolve(tok, macro)
+        name = resolve(tok, macro, localconst)
         if not name:
             continue
         line = text.count("\n", 0, m.start()) + 1
         out.append((name, "%s:%d" % (rel(path), line)))
     return out
 
 
 def harvest():
     macro = build_macros()
     found = {"excludeVars": collections.defaultdict(list),
              "cgiReads": collections.defaultdict(list),
              "cartRemoves": collections.defaultdict(list)}
     for path in source_files():
         try:
             text = open(path, errors="replace").read()
         except OSError:
             continue
         if "excludeVars" not in text and "cgi" not in text \
                 and "cartRemove" not in text:
             continue
         who = owner(path)
-        for name, src in find_exclude_vars(text, path, macro):
+        localconst = {m.group(1): m.group(2)[1:-1]
+                      for m in CONST_RE.finditer(text)}
+        for name, src in find_exclude_vars(text, path, macro, localconst):
             found["excludeVars"][who].append((name, src))
         for name, src in find_matches(CGI_READ_RE, text, path, macro,
-                                      CGI_READ_SKIP):
+                                      localconst, CGI_READ_SKIP):
             found["cgiReads"][who].append((name, src))
-        for name, src in find_matches(CART_REMOVE_RE, text, path, macro):
+        for name, src in find_matches(CART_REMOVE_RE, text, path, macro,
+                                      localconst):
             found["cartRemoves"][who].append((name, src))
     return found, macro
 
 
 # ---------------------------------------------------------------------------
 # reporting
 # ---------------------------------------------------------------------------
 
 def dedupe(pairs):
     """Collapse repeats of the same name, keeping the first site seen."""
     seen = {}
     for name, src in pairs:
         seen.setdefault(name, src)
     return sorted(seen.items())