Re: [PATCH] xdrgen: Handle typedef void types

[Chuck Lever <chuck.lever@oracle.com>]

Hi Khushal -

On 11/11/25 3:57 PM, Khushal Chitturi wrote:
> This patch Handle typedef void in XDR Generator. Previously, such
> declarations triggered a NotImplementedError in typedef.py.

Your patch adds a feature, but the patch description doesn't explain
why this change is needed. See below for an explanation of why
"void [identifier];" is not implemented.


> This change adds handling for _XdrVoid AST nodes within the
> typedef generator. When an XDR includes a typedef void,
> the generator now recognizes _XdrVoid nodes and emits the
> respective C typedefs and associated functions. New Jinja2
> templates were introduced for encoder, decoder, declaration,
> definition, and maxsize generation. The XDR grammar was
> updated so that void typedefs can be parsed properly
> 
> Tested by running xdrgen on tests/test.x containing a typedef void
> declaration. The tool now runs and produces the encoder, decoder,
> and typedef outputs across source, definitions, and declarations.
> 
> Signed-off-by: Khushal Chitturi <kc9282016@gmail.com>
> ---
>  tools/net/sunrpc/xdrgen/generators/typedef.py        | 12 ++++++++----
>  tools/net/sunrpc/xdrgen/grammars/xdr.lark            |  2 +-
>  .../xdrgen/templates/C/typedef/declaration/void.j2   |  2 ++
>  .../xdrgen/templates/C/typedef/decoder/void.j2       |  6 ++++++
>  .../xdrgen/templates/C/typedef/definition/void.j2    |  2 ++
>  .../xdrgen/templates/C/typedef/encoder/void.j2       |  6 ++++++
>  .../xdrgen/templates/C/typedef/maxsize/void.j2       |  2 ++
>  7 files changed, 27 insertions(+), 5 deletions(-)
>  create mode 100644 tools/net/sunrpc/xdrgen/templates/C/typedef/declaration/void.j2
>  create mode 100644 tools/net/sunrpc/xdrgen/templates/C/typedef/decoder/void.j2
>  create mode 100644 tools/net/sunrpc/xdrgen/templates/C/typedef/definition/void.j2
>  create mode 100644 tools/net/sunrpc/xdrgen/templates/C/typedef/encoder/void.j2
>  create mode 100644 tools/net/sunrpc/xdrgen/templates/C/typedef/maxsize/void.j2
> 
> diff --git a/tools/net/sunrpc/xdrgen/generators/typedef.py b/tools/net/sunrpc/xdrgen/generators/typedef.py
> index fab72e9d6915..f49ae26c4830 100644
> --- a/tools/net/sunrpc/xdrgen/generators/typedef.py
> +++ b/tools/net/sunrpc/xdrgen/generators/typedef.py
> @@ -58,7 +58,8 @@ def emit_typedef_declaration(environment: Environment, node: _XdrDeclaration) ->
>      elif isinstance(node, _XdrOptionalData):
>          raise NotImplementedError("<optional_data> typedef not yet implemented")
>      elif isinstance(node, _XdrVoid):
> -        raise NotImplementedError("<void> typedef not yet implemented")
> +        template = get_jinja2_template(environment, "declaration", node.template)
> +        print(template.render(name=node.name))
>      else:
>          raise NotImplementedError("typedef: type not recognized")
>  
> @@ -104,7 +105,8 @@ def emit_type_definition(environment: Environment, node: _XdrDeclaration) -> Non
>      elif isinstance(node, _XdrOptionalData):
>          raise NotImplementedError("<optional_data> typedef not yet implemented")
>      elif isinstance(node, _XdrVoid):
> -        raise NotImplementedError("<void> typedef not yet implemented")
> +        template = get_jinja2_template(environment, "definition", node.template)
> +        print(template.render(name=node.name))
>      else:
>          raise NotImplementedError("typedef: type not recognized")
>  
> @@ -165,7 +167,8 @@ def emit_typedef_decoder(environment: Environment, node: _XdrDeclaration) -> Non
>      elif isinstance(node, _XdrOptionalData):
>          raise NotImplementedError("<optional_data> typedef not yet implemented")
>      elif isinstance(node, _XdrVoid):
> -        raise NotImplementedError("<void> typedef not yet implemented")
> +        template = get_jinja2_template(environment, "decoder", node.template)
> +        print(template.render(name=node.name))
>      else:
>          raise NotImplementedError("typedef: type not recognized")
>  
> @@ -225,7 +228,8 @@ def emit_typedef_encoder(environment: Environment, node: _XdrDeclaration) -> Non
>      elif isinstance(node, _XdrOptionalData):
>          raise NotImplementedError("<optional_data> typedef not yet implemented")
>      elif isinstance(node, _XdrVoid):
> -        raise NotImplementedError("<void> typedef not yet implemented")
> +        template = get_jinja2_template(environment, "encoder", node.template)
> +        print(template.render(name=node.name))
>      else:
>          raise NotImplementedError("typedef: type not recognized")
>  

Looking at the _XdrVoid class definition (line 285-289 of xdr_ast.py):

  @dataclass
  class _XdrVoid(_XdrDeclaration):
      """A void declaration"""
      name: str = "void"  # Default value, never overridden
      template: str = "void"

The name field is never properly initialized with the (new) parsed
identifier. To implement this correctly, the void() transformer should
extract the identifier from children when present:

  def void(self, children):
      if children:
          name = children[0].symbol
          return _XdrVoid(name=name)
      return _XdrVoid()


> diff --git a/tools/net/sunrpc/xdrgen/grammars/xdr.lark b/tools/net/sunrpc/xdrgen/grammars/xdr.lark
> index 7c2c1b8c86d1..d8c5f7130d83 100644
> --- a/tools/net/sunrpc/xdrgen/grammars/xdr.lark
> +++ b/tools/net/sunrpc/xdrgen/grammars/xdr.lark
> @@ -8,7 +8,7 @@ declaration             : "opaque" identifier "[" value "]"            -> fixed_
>                          | type_specifier identifier "<" [ value ] ">"  -> variable_length_array
>                          | type_specifier "*" identifier                -> optional_data
>                          | type_specifier identifier                    -> basic
> -                        | "void"                                       -> void
> +                        | "void" [identifier] -> void

RFC 4506 defines void as a 0-byte quantity with declaration syntax
"void;" (no identifier), and void may only appear as union arms or in
program argument/result definitions.

The change void [identifier] violates this. RFC 4506 has errata (ID
6382) noting the grammar was too permissive, allowing void in incorrect
places—this change reintroduces that problem, allowing invalid
constructs like void foo; in struct bodies and typedefs.

Unless there's a strong reason to support "void [identifier];" I'm
going to have to reject the proposed feature. However, you might replace
the NotImplemented exception with something that better reflects that
the error is in the RPC language specification being parsed, not in the
xdrgen implementation.


>  value                   : decimal_constant
>                          | hexadecimal_constant
> diff --git a/tools/net/sunrpc/xdrgen/templates/C/typedef/declaration/void.j2 b/tools/net/sunrpc/xdrgen/templates/C/typedef/declaration/void.j2
> new file mode 100644
> index 000000000000..22c5226ee526
> --- /dev/null
> +++ b/tools/net/sunrpc/xdrgen/templates/C/typedef/declaration/void.j2
> @@ -0,0 +1,2 @@
> +{# SPDX-License-Identifier: GPL-2.0 #}
> +typedef void {{ name }};
> diff --git a/tools/net/sunrpc/xdrgen/templates/C/typedef/decoder/void.j2 b/tools/net/sunrpc/xdrgen/templates/C/typedef/decoder/void.j2
> new file mode 100644
> index 000000000000..ed9e2455b36f
> --- /dev/null
> +++ b/tools/net/sunrpc/xdrgen/templates/C/typedef/decoder/void.j2
> @@ -0,0 +1,6 @@
> +{# SPDX-License-Identifier: GPL-2.0 #}
> +static inline bool
> +xdrgen_decode_{{ name }}(struct xdr_stream *xdr, void *ptr)
> +{
> +    return true;

These templates are used to generate C code in the kernel style.
Indentation for generated code should use tabs, not spaces.


> +}
> diff --git a/tools/net/sunrpc/xdrgen/templates/C/typedef/definition/void.j2 b/tools/net/sunrpc/xdrgen/templates/C/typedef/definition/void.j2
> new file mode 100644
> index 000000000000..22c5226ee526
> --- /dev/null
> +++ b/tools/net/sunrpc/xdrgen/templates/C/typedef/definition/void.j2
> @@ -0,0 +1,2 @@
> +{# SPDX-License-Identifier: GPL-2.0 #}
> +typedef void {{ name }};
> diff --git a/tools/net/sunrpc/xdrgen/templates/C/typedef/encoder/void.j2 b/tools/net/sunrpc/xdrgen/templates/C/typedef/encoder/void.j2
> new file mode 100644
> index 000000000000..47d48af81546
> --- /dev/null
> +++ b/tools/net/sunrpc/xdrgen/templates/C/typedef/encoder/void.j2
> @@ -0,0 +1,6 @@
> +{# SPDX-License-Identifier: GPL-2.0 #}
> +static inline bool
> +xdrgen_encode_{{ name }}(struct xdr_stream *xdr, const void *ptr)
> +{
> +    return true;

Ditto.


> +}
> diff --git a/tools/net/sunrpc/xdrgen/templates/C/typedef/maxsize/void.j2 b/tools/net/sunrpc/xdrgen/templates/C/typedef/maxsize/void.j2
> new file mode 100644
> index 000000000000..129374200ad0
> --- /dev/null
> +++ b/tools/net/sunrpc/xdrgen/templates/C/typedef/maxsize/void.j2
> @@ -0,0 +1,2 @@
> +{# SPDX-License-Identifier: GPL-2.0 #}
> +#define {{ macro }} 0

The convention for these size macros is to wrap their expanded value
with parentheses to prevent them from being unintentionally combined
with other tokens after expansion.


-- 
Chuck Lever