All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <cel@kernel.org>
To: NeilBrown <neil@brown.name>, Jeff Layton <jlayton@kernel.org>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: <linux-nfs@vger.kernel.org>
Subject: [PATCH 3/5] xdrgen: Reject specifications that define a name twice
Date: Sun, 12 Jul 2026 16:34:49 -0400	[thread overview]
Message-ID: <20260712203451.124902-4-cel@kernel.org> (raw)
In-Reply-To: <20260712203451.124902-1-cel@kernel.org>

When an RPC specification defines the same type or constant name
more than once, currently xdrgen emits every definition without
complaint. The duplication surfaces later as a C compiler error
about a redefined struct or function that points at generated code
instead of the actual offending line in the .x source.

RFC 4506 Section 6.4 places constant and type identifiers in a
single name space that must be unique within a specification. Add
a semantic check that enforces this rule.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 .../net/sunrpc/xdrgen/subcmds/declarations.py | 10 +--
 .../net/sunrpc/xdrgen/subcmds/definitions.py  |  7 ++-
 tools/net/sunrpc/xdrgen/subcmds/lint.py       |  7 ++-
 tools/net/sunrpc/xdrgen/subcmds/source.py     |  7 ++-
 tools/net/sunrpc/xdrgen/xdr_ast.py            | 61 ++++++++++++++++++-
 tools/net/sunrpc/xdrgen/xdr_parse.py          | 20 ++++++
 6 files changed, 101 insertions(+), 11 deletions(-)

diff --git a/tools/net/sunrpc/xdrgen/subcmds/declarations.py b/tools/net/sunrpc/xdrgen/subcmds/declarations.py
index ed83d48d1f68..f187611466d7 100644
--- a/tools/net/sunrpc/xdrgen/subcmds/declarations.py
+++ b/tools/net/sunrpc/xdrgen/subcmds/declarations.py
@@ -21,16 +21,15 @@ from generators.union import XdrUnionGenerator
 
 from xdr_ast import transform_parse_tree, _RpcProgram, Specification
 from xdr_ast import _XdrEnum, _XdrPointer, _XdrTypedef, _XdrStruct, _XdrUnion
+from xdr_ast import XdrSemanticError
 from xdr_parse import xdr_parser, set_xdr_annotate
 from xdr_parse import make_error_handler, XdrParseError
-from xdr_parse import handle_transform_error
+from xdr_parse import handle_transform_error, handle_semantic_error
 
 logger.setLevel(logging.INFO)
 
 
-def emit_header_declarations(
-    root: Specification, language: str, peer: str
-) -> None:
+def emit_header_declarations(root: Specification, language: str, peer: str) -> None:
     """Emit header declarations"""
     for definition in root.definitions:
         if isinstance(definition.value, _XdrEnum):
@@ -68,6 +67,9 @@ def subcmd(args: Namespace) -> int:
         except VisitError as e:
             handle_transform_error(e, source, args.filename)
             return 1
+        except XdrSemanticError as e:
+            handle_semantic_error(e, source, args.filename)
+            return 1
 
         gen = XdrHeaderTopGenerator(args.language, args.peer)
         gen.emit_declaration(args.filename, ast)
diff --git a/tools/net/sunrpc/xdrgen/subcmds/definitions.py b/tools/net/sunrpc/xdrgen/subcmds/definitions.py
index a48ca0549382..77b666943a11 100644
--- a/tools/net/sunrpc/xdrgen/subcmds/definitions.py
+++ b/tools/net/sunrpc/xdrgen/subcmds/definitions.py
@@ -21,12 +21,12 @@ from generators.typedef import XdrTypedefGenerator
 from generators.struct import XdrStructGenerator
 from generators.union import XdrUnionGenerator
 
-from xdr_ast import transform_parse_tree, Specification
+from xdr_ast import transform_parse_tree, Specification, XdrSemanticError
 from xdr_ast import _RpcProgram, _XdrConstant, _XdrEnum, _XdrPassthru, _XdrPointer
 from xdr_ast import _XdrTypedef, _XdrStruct, _XdrUnion
 from xdr_parse import xdr_parser, set_xdr_annotate
 from xdr_parse import make_error_handler, XdrParseError
-from xdr_parse import handle_transform_error
+from xdr_parse import handle_transform_error, handle_semantic_error
 
 logger.setLevel(logging.INFO)
 
@@ -94,6 +94,9 @@ def subcmd(args: Namespace) -> int:
         except VisitError as e:
             handle_transform_error(e, source, args.filename)
             return 1
+        except XdrSemanticError as e:
+            handle_semantic_error(e, source, args.filename)
+            return 1
 
         gen = XdrHeaderTopGenerator(args.language, args.peer)
         gen.emit_definition(args.filename, ast)
diff --git a/tools/net/sunrpc/xdrgen/subcmds/lint.py b/tools/net/sunrpc/xdrgen/subcmds/lint.py
index e1da49632e62..b4ea0f55f079 100644
--- a/tools/net/sunrpc/xdrgen/subcmds/lint.py
+++ b/tools/net/sunrpc/xdrgen/subcmds/lint.py
@@ -11,8 +11,8 @@ from lark import logger
 from lark.exceptions import VisitError
 
 from xdr_parse import xdr_parser, make_error_handler, XdrParseError
-from xdr_parse import handle_transform_error
-from xdr_ast import transform_parse_tree
+from xdr_parse import handle_transform_error, handle_semantic_error
+from xdr_ast import transform_parse_tree, XdrSemanticError
 
 logger.setLevel(logging.DEBUG)
 
@@ -34,5 +34,8 @@ def subcmd(args: Namespace) -> int:
         except VisitError as e:
             handle_transform_error(e, source, args.filename)
             return 1
+        except XdrSemanticError as e:
+            handle_semantic_error(e, source, args.filename)
+            return 1
 
     return 0
diff --git a/tools/net/sunrpc/xdrgen/subcmds/source.py b/tools/net/sunrpc/xdrgen/subcmds/source.py
index 27e8767b1b58..56eba34d8eb3 100644
--- a/tools/net/sunrpc/xdrgen/subcmds/source.py
+++ b/tools/net/sunrpc/xdrgen/subcmds/source.py
@@ -21,11 +21,11 @@ from generators.union import XdrUnionGenerator
 
 from xdr_ast import transform_parse_tree, _RpcProgram, Specification
 from xdr_ast import _XdrAst, _XdrEnum, _XdrPassthru, _XdrPointer
-from xdr_ast import _XdrStruct, _XdrTypedef, _XdrUnion
+from xdr_ast import _XdrStruct, _XdrTypedef, _XdrUnion, XdrSemanticError
 
 from xdr_parse import xdr_parser, set_xdr_annotate, set_xdr_enum_validation
 from xdr_parse import make_error_handler, XdrParseError
-from xdr_parse import handle_transform_error
+from xdr_parse import handle_transform_error, handle_semantic_error
 
 logger.setLevel(logging.INFO)
 
@@ -123,6 +123,9 @@ def subcmd(args: Namespace) -> int:
         except VisitError as e:
             handle_transform_error(e, source, args.filename)
             return 1
+        except XdrSemanticError as e:
+            handle_semantic_error(e, source, args.filename)
+            return 1
         match args.peer:
             case "server":
                 generate_server_source(args.filename, ast, args.language)
diff --git a/tools/net/sunrpc/xdrgen/xdr_ast.py b/tools/net/sunrpc/xdrgen/xdr_ast.py
index 15d2c6c40dd9..cf68ff5dfe17 100644
--- a/tools/net/sunrpc/xdrgen/xdr_ast.py
+++ b/tools/net/sunrpc/xdrgen/xdr_ast.py
@@ -814,7 +814,9 @@ def _merge_consecutive_passthru(definitions: List[Definition]) -> List[Definitio
             lines = [definitions[i].value.content]
             meta = definitions[i].meta
             j = i + 1
-            while j < len(definitions) and isinstance(definitions[j].value, _XdrPassthru):
+            while j < len(definitions) and isinstance(
+                definitions[j].value, _XdrPassthru
+            ):
                 lines.append(definitions[j].value.content)
                 j += 1
             merged = _XdrPassthru("\n".join(lines))
@@ -826,10 +828,67 @@ def _merge_consecutive_passthru(definitions: List[Definition]) -> List[Definitio
     return result
 
 
+def _meta_line(meta) -> int:
+    """Return the 1-based source line for a node's meta, or 0 if unknown"""
+    try:
+        return meta.line
+    except AttributeError:
+        return 0
+
+
+class XdrSemanticError(Exception):
+    """A specification that parses but violates an XDR semantic rule.
+
+    Detection lives in the language-independent front end because a
+    duplicate name is malformed XDR regardless of the output language.
+    """
+
+    def __init__(self, message: str, meta):
+        super().__init__(message)
+        self.message = message
+        self.line = _meta_line(meta)
+        self.column = getattr(meta, "column", 0)
+
+
+def _introduced_names(value):
+    """Yield (name, node) for each identifier a definition introduces."""
+    if isinstance(value, (_XdrStruct, _XdrUnion, _XdrPointer)):
+        yield value.name, value
+    elif isinstance(value, _XdrEnum):
+        yield value.name, value
+        for enumerator in value.enumerators:
+            yield enumerator.name, enumerator
+    elif isinstance(value, _XdrTypedef):
+        yield value.declaration.name, value.declaration
+    elif isinstance(value, _XdrConstant):
+        yield value.name, value
+
+
+def check_duplicate_definitions(root: "Specification") -> None:
+    """Reject a spec that declares an identifier more than once.
+
+    RFC 4506 Section 6.4 places constant and type identifiers in a
+    single name space that must be unique within a specification.
+    """
+    seen = {}
+    for definition in root.definitions:
+        for name, node in _introduced_names(definition.value):
+            where = node if node.line else definition.meta
+            first = seen.get(name)
+            if first is not None:
+                raise XdrSemanticError(
+                    f"duplicate identifier '{name}'"
+                    f" (first declared at line {_meta_line(first)})",
+                    where,
+                )
+            seen[name] = where
+
+
 def transform_parse_tree(parse_tree):
     """Transform productions into an abstract syntax tree"""
     ast = transformer.transform(parse_tree)
     ast.definitions = _merge_consecutive_passthru(ast.definitions)
+    check_duplicate_definitions(ast)
     return ast
 
 
diff --git a/tools/net/sunrpc/xdrgen/xdr_parse.py b/tools/net/sunrpc/xdrgen/xdr_parse.py
index 3e76717d2f85..78298553ee78 100644
--- a/tools/net/sunrpc/xdrgen/xdr_parse.py
+++ b/tools/net/sunrpc/xdrgen/xdr_parse.py
@@ -169,6 +169,26 @@ def handle_transform_error(e: VisitError, source: str, filename: str) -> None:
     sys.stderr.write("\n".join(msg_parts) + "\n")
 
 
+def handle_semantic_error(e, source: str, filename: str) -> None:
+    """Report a semantic error (e.g., a duplicate name) with context.
+
+    Args:
+        e: The XdrSemanticError carrying message and source position
+        source: The XDR source text being parsed
+        filename: The name of the file being parsed
+    """
+    lines = source.splitlines()
+    line_num = getattr(e, "line", 0)
+    column = getattr(e, "column", 0)
+    line_text = lines[line_num - 1] if 0 < line_num <= len(lines) else ""
+
+    msg_parts = [f"{filename}:{line_num}:{column}: semantic error", e.message]
+    if line_text:
+        msg_parts.extend(format_source_caret(line_text, column))
+
+    sys.stderr.write("\n".join(msg_parts) + "\n")
+
+
 def xdr_parser() -> Lark:
     """Return a Lark parser instance configured with the XDR language grammar"""
 
-- 
2.54.0


  parent reply	other threads:[~2026-07-12 20:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12 20:34 [PATCH 0/5] xdrgen: Improve diagnostic reporting Chuck Lever
2026-07-12 20:34 ` [PATCH 1/5] xdrgen: Align the error caret under tab-indented source Chuck Lever
2026-07-12 20:34 ` [PATCH 2/5] xdrgen: Record the source position of each declared identifier Chuck Lever
2026-07-12 20:34 ` Chuck Lever [this message]
2026-07-12 20:34 ` [PATCH 4/5] xdrgen: Enforce RFC 5531 name and number scoping for RPC programs Chuck Lever
2026-07-12 20:34 ` [PATCH 5/5] xdrgen: Reject out-of-range program, version, and procedure numbers Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260712203451.124902-4-cel@kernel.org \
    --to=cel@kernel.org \
    --cc=dai.ngo@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.