Re: [PATCH v3 3/5] perf synthetic-events: Ensure features are aligned

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Dec 16, 2024 at 03:24:57PM -0800, Ian Rogers wrote:
> Features like hostname have arbitrary size and break the assumed
> 8-byte alignment of perf events. Pad all feature events until 8-byte
> alignment is restored.

But it seems write_hostname() and others use do_write_string() which
handles the padding already.

thanks,
Namhyung

> 
> Rather than invent a new mechanism, reuse write_padded but pass an
> empty buffer as what to write. As no alignment may be necessary,
> update write_padded to be tolerant of 0 as a count and count_aligned
> value.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/header.c           | 7 ++++---
>  tools/perf/util/synthetic-events.c | 2 ++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 03e43a9894d4..d10703090e83 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -168,11 +168,12 @@ int write_padded(struct feat_fd *ff, const void *bf,
>  		 size_t count, size_t count_aligned)
>  {
>  	static const char zero_buf[NAME_ALIGN];
> -	int err = do_write(ff, bf, count);
> +	int err = count > 0 ? do_write(ff, bf, count) : 0;
>  
> -	if (!err)
> +	if (!err && count_aligned > count) {
> +		assert(count_aligned - count < sizeof(zero_buf));
>  		err = do_write(ff, zero_buf, count_aligned - count);
> -
> +	}
>  	return err;
>  }
>  
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index f8ac2ac2da45..ee6e9c2fb11b 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -2401,6 +2401,8 @@ int perf_event__synthesize_features(const struct perf_tool *tool, struct perf_se
>  			pr_debug("Error writing feature\n");
>  			continue;
>  		}
> +		write_padded(&ff, /*bf=*/NULL, /*count=*/0,
> +			     /*count_aligned=*/PERF_ALIGN(ff.offset, sizeof(u64)) - ff.offset);
>  		/* ff.buf may have changed due to realloc in do_write() */
>  		fe = ff.buf;
>  		memset(fe, 0, sizeof(*fe));
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
>