Linux NFS development
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox