Re: [DTrace-devel] [PATCH v2 3/3] Add more robust mechanism to skip tracepoint common fields

On 13 Sep 2025, eugene loh stated:
> Implement a more robust mechanism.
>
> Specifically, instead of skipping a hardwired (SKIP_FIELDS_COUNT=4)
> number of common fields, look for "common_" names.  E.g., in
> kernel/trace/trace_events.c in trace_define_common_fields(), we
> see the macro __common_field() is used to define common fields,
> and the names are prepended with "common_".

(If this turns out not to work, maybe we can hunt for the blank line
that always seems to follow common_* fields?)

>  	while (getline(&buf, &bufsz, f) >= 0) {

This is much clearer than it was before!

Reviewed-by: Nick Alcock <[email protected]>

> @@ -160,14 +150,63 @@ dt_tp_event_info(dtrace_hdl_t *dtp, FILE *f, int skip, tp_probe_t *tpp,
>  
>  		if (sscanf(buf, "ID: %u\n", &tpp->id) == 1)
>  			continue;
> -
>  		if (sscanf(buf, " field:%[^;]", p) <= 0)
>  			continue;
> -		sscanf(p, "__data_loc %[^;]", p);
>  
> -		/* We found a field: description - see if we should skip it. */
> -		if (argc++ < 0)
> -			continue;
> +		/*
> +		 * If we have only seen common fields to date, keep
> +		 * looking for a non-common field.
> +		 */
> +		if (common == 1) {

... ok, I take back the thing I said in the previous mail about moving
things to a separate loop: the ID/field scan is necessary anyway, given
the first two lines:

ID: 94
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;

Given the overall format of e.g.

name: sys_enter_iopl
ID: 94
format:
	field:unsigned short common_type;	offset:0;	size:2;	signed:0;
	field:unsigned char common_flags;	offset:2;	size:1;	signed:0;
	field:unsigned char common_preempt_count;	offset:3;	size:1;	signed:0;
	field:int common_pid;	offset:4;	size:4;	signed:1;

	field:int __syscall_nr;	offset:8;	size:4;	signed:1;

an alternative approach might be to exploit the format, and always skip
all lines that do not start with a tab, and/or simply all lines until
the first entirely blank line? Maybe? (Looking at the kernel source, the
common fields always seem to be followed by a blank one.)

But this approach will work for now, though it appears to be pure
coincidence that all common fields start with common_. We definitely
have workable alternative approaches if this stops working :)

Hmm...

+			/*
+			 * Strip off any [] array size specifications at the end.
+			 */
+			while (*s == ']') {
+				/* From ']' hunt back to '['.  They are not nested. */
+				while (s > p && *(--s) != '[') ;
+
+				/* Then remove any spaces. */
+				while (s > p && *(--s) == ' ') ;
+			}
+			*(++s) = '\0';

Why is this necessary? We never look at the end of the identifier in the
common_ search, only the start...