Re: [PATCH v2 3/4] perf tools: Convert to perf_record_header_attr_id()

[Adrian Hunter <adrian.hunter@intel.com>]

On 30/08/23 05:15, Namhyung Kim wrote:
> Hi Adrian,
> 
> On Mon, Aug 28, 2023 at 12:59 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 25/08/23 18:25, Namhyung Kim wrote:
>>> Instead of accessing the attr.id directly, use the
>>> perf_record_header_attr_id() helper to handle old versions.
>>>
>>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>>> ---
>>>  tools/perf/util/header.c           | 2 +-
>>>  tools/perf/util/session.c          | 4 ++--
>>>  tools/perf/util/synthetic-events.c | 2 +-
>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>>> index f89321cbfdee..63967c34157d 100644
>>> --- a/tools/perf/util/header.c
>>> +++ b/tools/perf/util/header.c
>>> @@ -4408,7 +4408,7 @@ int perf_event__process_attr(struct perf_tool *tool __maybe_unused,
>>>       if (perf_evsel__alloc_id(&evsel->core, 1, n_ids))
>>>               return -ENOMEM;
>>>
>>> -     ids = (void *)&event->attr.attr + event->attr.attr.size;
>>> +     ids = perf_record_header_attr_id(event);
>>>       for (i = 0; i < n_ids; i++) {
>>>               perf_evlist__id_add(&evlist->core, &evsel->core, 0, i, ids[i]);
>>>       }
>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>> index 00d18c74c090..1e9aa8ed15b6 100644
>>> --- a/tools/perf/util/session.c
>>> +++ b/tools/perf/util/session.c
>>> @@ -833,8 +833,8 @@ static void perf_event__hdr_attr_swap(union perf_event *event,
>>>       perf_event__attr_swap(&event->attr.attr);
>>>
>>>       size = event->header.size;
>>> -     size -= (void *)&event->attr.id - (void *)event;
>>> -     mem_bswap_64(event->attr.id, size);
>>> +     size -= perf_record_header_attr_id(event) - (void *)event;
>>> +     mem_bswap_64(perf_record_header_attr_id(event), size);
>>
>> Should this and below also be a fix for stable?
> 
> Then it'd need the perf_record_header_attr_id() change as well.
> Also I'm not sure if it's really needed for stable as it's a pipe output
> (saved to a file though) so very unlikely to have a different endian.
> 
> Thanks,
> Namhyung

Ok

For the patch set:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> 
>>
>>>  }
>>>
>>>  static void perf_event__event_update_swap(union perf_event *event,
>>> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
>>> index 45714a2785fd..a0579c7d7b9e 100644
>>> --- a/tools/perf/util/synthetic-events.c
>>> +++ b/tools/perf/util/synthetic-events.c
>>> @@ -2145,7 +2145,7 @@ int perf_event__synthesize_attr(struct perf_tool *tool, struct perf_event_attr *
>>>               return -ENOMEM;
>>>
>>>       ev->attr.attr = *attr;
>>> -     memcpy(ev->attr.id, id, ids * sizeof(u64));
>>> +     memcpy(perf_record_header_attr_id(ev), id, ids * sizeof(u64));
>>>
>>>       ev->attr.header.type = PERF_RECORD_HEADER_ATTR;
>>>       ev->attr.header.size = (u16)size;
>>