Re: [PATCH bpf-next v4 1/6] bpf: Correct the overflow check comment in bpf_iter_num_next()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added multi-line comment in `bpf_iter_num_next()` does not follow the required BPF subsystem comment style.
--

--- Patch [1]: [PATCH bpf-next v4 1/6] bpf: Correct the overflow check comment in bpf_iter_num_next() ---
commit 385d10f67ceb46ec31888afb23333c5f7645c9f6
Author: Puranjay Mohan <puranjay@kernel.org>

bpf: Correct the overflow check comment in bpf_iter_num_next()

This commit removes a redundant 64-bit cast in the bpf_iter_num_next()
iterator loop and rewrites the associated comment to correctly describe
that the 32-bit wraparound is intentional for handling starts at INT_MIN.

> diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> index f5eaeb2493d4a..f190f2b250048 100644
> --- a/kernel/bpf/bpf_iter.c
> +++ b/kernel/bpf/bpf_iter.c
> @@ -802,12 +802,14 @@ __bpf_kfunc int *bpf_iter_num_next(struct bpf_iter_num* it)
>  {
>  	struct bpf_iter_num_kern *s = (void *)it;
>  
> -	/* check failed initialization or if we are done (same behavior);
> -	 * need to be careful about overflow, so convert to s64 for checks,
> -	 * e.g., if s->cur == s->end == INT_MAX, we can't just do
> -	 * s->cur + 1 >= s->end
> +	/* Detect the end of the range, or a failed/empty iterator: all of these

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the preferred BPF
subsystem style?

The BPF guidelines specify that multi-line comments should have the opening
/* on its own line, with the text starting on the next line. Since this
comment block is being rewritten entirely, would it be better to update it
to match the modern multi-line comment format?

> +	 * leave s->cur + 1 >= s->end. bpf_iter_num_new() set s->cur to start - 1
> +	 * (which wraps to INT_MAX when start == INT_MIN), so the s->cur + 1 below
> +	 * is a deliberate 32-bit wraparound that recovers start. As s->cur and
> +	 * s->end are int, this is an ordinary signed 32-bit compare, exactly what
> +	 * the inlined bpf_iter_num_next() emits.
>  	 */
> -	if ((s64)(s->cur + 1) >= s->end) {
> +	if (s->cur + 1 >= s->end) {
>  		s->cur = s->end = 0;
>  		return NULL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729203633.213973-1-puranjay@kernel.org?part=1