Re: [PATCH v2 2/2] libtraceevent: Pretty-print cpumask fields as a cpulist

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 16 Nov 2022 14:46:46 +0000
Valentin Schneider <vschneid@redhat.com> wrote:

> +static void print_cpumask_to_seq(struct tep_handle *tep,
> +				 struct trace_seq *s, const char *format,
> +				 int len_arg, const void *data, int size)
> +{
> +	int firstone = -1, firstzero = -1;
> +	int nr_bits = size * 8;
> +	bool first = true;
> +	int str_size = 0;
> +	char buf[12]; /* '-' + log10(2^32) + 1 digits + '\0' */
> +	char *str;
> +	int index;
> +	int i;
> +
> +	str = malloc(cpumask_worst_size(nr_bits) + 1);
> +	if (!str) {
> +		do_warning("%s: not enough memory!", __func__);
> +		return;
> +	}
> +
> +	for (i = 0; i < size; i++) {
> +		unsigned char byte;
> +		int fmtsize;
> +
> +		if (tep->file_bigendian)
> +			index = size - (i + 1);
> +		else
> +			index = i;
> +
> +		/* Byte by byte scan, not the best... */
> +		byte = *(((unsigned char *)data) + index);
> +more:
> +		/* First find a bit set to one...*/
> +		if (firstone < 0 && byte) {
> +			/*
> +			 * Set all lower bits, so a later ffz on this same byte
> +			 * is guaranteed to find a later bit.
> +			 */
> +			firstone = ffs(byte) - 1;
> +			byte |= (1 << firstone) - 1;
> +			firstone += i * 8;
> +		}
> +
> +		if (firstone < 0)
> +			continue;
> +
> +		/* ...Then find a bit set to zero */
> +		if ((~byte) & 0xFF) {
> +			/*
> +			 * Clear all lower bits, so a later ffs on this same
> +			 * byte is guaranteed to find a later bit.
> +			 */
> +			firstzero = ffs(~byte) - 1;
> +			byte &= ~((1 << (firstzero)) - 1);
> +			firstzero += i * 8;
> +		} else if (i == size - 1) { /* ...Or reach the end of the mask */
> +			firstzero = nr_bits;
> +			byte = 0;
> +		} else {
> +			continue;
> +		}
> +
> +		/* We've found a bit set to one, and a later bit set to zero. */
> +		if (!first) {
> +			str[str_size] = ',';
> +			str_size++;
> +		}
> +		first = false;
> +
> +		/* It takes {log10(number) + 1} chars to format a number */
> +		fmtsize = log10(firstone) + 1;
> +		snprintf(buf, fmtsize + 1, "%d", firstone);
> +		memcpy(str + str_size, buf, fmtsize);
> +		str_size += fmtsize;
> +
> +		if (firstzero > firstone + 1) {
> +			fmtsize = log10(firstzero - 1) + 2;
> +			snprintf(buf, fmtsize + 1, "-%d", firstzero - 1);
> +			memcpy(str + str_size, buf, fmtsize);
> +			str_size += fmtsize;
> +		}
> +
> +		firstzero = firstone = -1;
> +		if (byte)
> +			goto more;
> +	}
> +
> +	str[str_size] = 0;
> +	str_size++;
> +
> +	if (len_arg >= 0)
> +		trace_seq_printf(s, format, len_arg, str);
> +	else
> +		trace_seq_printf(s, format, str);
> +
> +	free(str);
> +}
> +


This is a rather complex algorithm (I'm too tired to try to grasp it). It
really needs a unit test to make sure it's working as expected.

-- Steve