Re: [PATCH RFC 05/20] ctlv: Prepare message input handling

[Jonathan Cameron]

On Thu, 15 Oct 2026 16:50:38 +0200
Jiri Pirko <jiri@xxxxxxxxxxx> wrote:

> From: Jiri Pirko <jiri@xxxxxxxxxx>
> 
> Add wire walking, request indexing and validated input accessors for
> generated family readers.
> 
> Keep CONFIG_CTLV unwired until dispatch completes the module.
> 
> Assisted-by: Cursor: GPT-5.6 Sol
> Assisted-by: Cursor: Claude Opus 5.5
> Signed-off-by: Jiri Pirko <jiri@xxxxxxxxxx>

Trivial stuff only. I didn't attempt (yet) to check
all the walking code, just get a feeling for what was here. 


> diff --git a/drivers/ctlv/Kconfig b/drivers/ctlv/Kconfig
> new file mode 100644
> index 000000000000..aed3243c55fd
> --- /dev/null
> +++ b/drivers/ctlv/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menuconfig CTLV
> +	tristate "CTLV character device TLV UAPI"
> +	help
> +	  CTLV provides character device UAPIs described by a checked-in YAML
> +	  specification. It owns the character device lifetime, the strict TLV
> +	  message format, request validation, reply serialization, the large
> +	  payloads a request describes rather than carries, per-open event
> +	  queues and runtime schema introspection, so families only implement
> +	  semantics.
> +
> +	  If you don't know what to do here, say N.

I'd not say that.  Overly cautious for something we expect
to get fairly extensive use.


> diff --git a/drivers/ctlv/message.c b/drivers/ctlv/message.c
> new file mode 100644
> index 000000000000..7e5742ba8f70
> --- /dev/null
> +++ b/drivers/ctlv/message.c

> +
> +bool ctlv_message_input_rebind(struct ctlv_cursor *nest, const struct ctlv_schema_tree *tree,
> +			       u32 node)
> +{
> +	const struct ctlv_message_input_scope *bound_scope;
> +	const struct ctlv_message_input_nest *indexed;
> +	u32 i;
> +
> +	if (WARN_ON_ONCE(!nest->attr || !nest->msg || !nest->msg->index))
> +		return false;
> +	if (!ctlv_message_nest_index_ok(nest))
> +		return false;
> +	bound_scope = &nest->msg->index->scopes[nest->index_nest_scope];
> +	indexed = &nest->msg->index->nests[bound_scope->nest];
> +	for (i = 0; i < indexed->scopes_count; i++) {

	for (u32 i = 0; ...

> +		u32 scope_index = indexed->first_scope + i;
> +		const struct ctlv_message_input_scope *scope =
> +			&nest->msg->index->scopes[scope_index];
> +
> +		if (scope->tree != tree || scope->node != node)
> +			continue;
> +		nest->tree = tree;
> +		nest->node = node;
> +		nest->index_nest_scope = scope_index;
> +		return true;
> +	}
> +	return false;
> +}

> +
> +void ctlv_attr_next(struct ctlv_cursor *cursor)
> +{
> +	const struct ctlv_message_input_slot *slot;
> +	const struct ctlv_message_input_scope *scope;
> +	const struct ctlv_node_schema *node;
> +	const struct ctlv_attr *next;
> +	u32 next_nest;
> +	u32 next_offset;
> +
> +	slot = ctlv_message_cursor_index(cursor);
> +	if (!slot || WARN_ON_ONCE(!cursor->attr))
> +		return;
> +	next = (const void *)((const u8 *)cursor->attr +
> +			      ALIGN((size_t)cursor->attr->len, CTLV_ALIGNTO));

kernel code is allowed to rely on void * arithmetic, so this can be
	next = (const void *)cursor->attr +
	       ALIGN((size_t)cursor->attr->len, CTLV_ALIGNTO);

container_of() for instance makes use of sizeof(void) == 1.
 


> +const void *ctlv_attr_binary(const struct ctlv_cursor *cursor, u32 *len)
> +{
> +	const void *value;
> +
> +	*len = 0;
> +	value = ctlv_message_cursor_payload(cursor, BIT(CTLV_SCHEMA_BINARY), len);
> +	if (!value)
> +		*len = 0;

I'd kind of have assumed that ctlv_message_cursor_payload() would not
set len to anything if it returns NULL.  Thus the default *len = 0 would
be intact.  From a quick look that seems to be the case making this
code a little odd. 

Perhaps overly defensive or keeping an LLM happy?
Same applies on some other functions.



> +	return value;
> +}
> +EXPORT_SYMBOL_NS_GPL(ctlv_attr_binary, "CTLV");
> +
> +/*
> + * The room is measured before anything is written, which is what leaves a
> + * refused attr's message exactly as it was, padding included.
> + */

Having a trailing comment in a file rarely adds much or indeed
even more rarely gets read.


> diff --git a/drivers/ctlv/message.h b/drivers/ctlv/message.h