Re: git: 3d73146baeb9 - main - pwait: Add an option to print remaining processes

Zhenlei Huang <zlei_at_FreeBSD.org> wrote on
Date: Tue, 28 Oct 2025 18:18:55 UTC :

> > On Oct 29, 2025, at 1:28 AM, Gleb Smirnoff <glebius@freebsd.org> wrote:
> > 
> > On Tue, Oct 28, 2025 at 11:57:43AM +0000, Dag-Erling Smørgrav wrote:
> > D> +static int
> > D> +pidcmp(const struct pid *a, const struct pid *b)
> > D> +{
> > D> + return (a->pid > b->pid ? 1 : a->pid < b->pid ? -1 : 0);
> > D> +}
> > D> +
> > D> +RB_HEAD(pidtree, pid);
> > D> +static struct pidtree pids = RB_INITIALIZER(&pids);
> > D> +RB_GENERATE_STATIC(pidtree, pid, entry, pidcmp);
> > 
> > We have a nice trick in our tree(3) that allows to use lighter compare
> > functions. The function can return any signed integer type, thus we
> > can:
> > 
> > static pid_t
> > pidcmp(const struct pid *a, const struct pid *b)
> > {
> > return (a->pid - b->pid);
> > }
> 
> I'd prefer to return const 1 / -1 / 0, that is straight forward of a comparator. Also the compiler is smart
> enough to catch this pattern and generate optimized code, from my experiment which is long long time ago.
> 
> The pid has type pid_t which is a type redefinition of __int32_t. Although the pid will not reach
> 2^31 or -2^31 - 1, in principle the computing of the delta of the two signed integers may overflow
> and that is bad smell.

Dag-Erling's code is somewhat less dependent on the
context of use, at least relative to the C11 language
standard definitions.


FreeBSD sometimes defines that it requires properties not
guaranteed by language or other standards. So I'm not sure
the "a->pid - b->pid" code in question is just depending
on such a definition vs. not. Also, it may be that all the
calls of pidcmp meet the criteria that I reference below.

QUOTE (of C11 standard)
When two pointers are subtracted, both shall point to
elements of the same array object or one past the last
element of the array object; the result is the difference
of the subscripts of the two array elements. The size of
the result is implementation-defined, and its type (a signed
integer type) is ptrdiff_t . . . If the result is not
representable in an object of that type, the behavior is
undefined.
END QUOTE

So, making some language context more explicit, the return
is more like:

return (pid_t)(ptrdiff_t)(a->pid - b->pid);

If pid_t is __int32_t even for 64 bit environments,
the relationship to ptrdiff_t for the context could
be a technical problem. I'm not sure if FreeBSD makes
any guarantees that establish a sufficient relationship.

Also, the above does not allow subtracting pointers to
distinct members of the same struct, for example.


Contrast with a->pid > b->pid and a->pid < b->pid use:

The C11 standard wording covering a->pid > b->pid and
a->pid < b->pid from the committed code actually allows
a wider range of valid-to-compare pointer values, such
as for pointers to of members of the same struct.

But without being members of the same array or
struct or union, the comparisons can be examples of
"[i]n all other cases, the behavior is undefined".

So what the language allows on its own is still
limited.


(My notes may span somewhat more than is needed for the
context.)


===
Mark Millard
marklmi at yahoo.com