Re: [PATCH v6 16/18] perf: enable reading of perf.data files from different ABI rev

[David Ahern <dsahern@gmail.com>]

On 02/09/2012 03:41 PM, Stephane Eranian wrote:
>>> +     /* read perf_file_section, ids are read in caller */
>>> +     ret = readn(fd, &f_attr->ids, sizeof(f_attr->ids));
>>
>> I thought the above 2 lines were to be removed.
>>
> 
> No, They are needed to get the offsets of the ids[] table.
> The field name are confusing. This is not reading the ids[],
> but just the offset of where the table is. Without that you
> get bogus offsets. I ran into that bug during testing...

Hmm.... ok, so the perf_file_attr is now read in back to back reads --
attr then ids -- versus today where it gets both in one readn.


> 
> 
>> David
>>
>>
>>> +
>>> +     return ret <= 0 ? -1 : 0;
>>> +}
>>> +
>>>  int perf_session__read_header(struct perf_session *session, int fd)
>>>  {
>>>       struct perf_header *header = &session->header;
>>> @@ -1974,19 +2019,17 @@ int perf_session__read_header(struct perf_session *session, int fd)
>>>       if (session->fd_pipe)
>>>               return perf_header__read_pipe(session, fd);
>>>
>>> -     if (perf_file_header__read(&f_header, header, fd) < 0) {
>>> -             pr_debug("incompatible file format\n");
>>> +     if (perf_file_header__read(&f_header, header, fd) < 0)
>>>               return -EINVAL;
>>> -     }
>>>
>>> -     nr_attrs = f_header.attrs.size / sizeof(f_attr);
>>> +     nr_attrs = f_header.attrs.size / f_header.attr_size;
>>>       lseek(fd, f_header.attrs.offset, SEEK_SET);
>>>
>>>       for (i = 0; i < nr_attrs; i++) {
>>>               struct perf_evsel *evsel;
>>>               off_t tmp;
>>>
>>> -             if (readn(fd, &f_attr, sizeof(f_attr)) <= 0)
>>> +             if (read_attr(fd, header, &f_attr) < 0)
>>>                       goto out_errno;
>>>
>>>               if (header->needs_swap)
>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>> index 3f3afed..5476320 100644
>>> --- a/tools/perf/util/session.c
>>> +++ b/tools/perf/util/session.c
>>> @@ -24,7 +24,7 @@ static int perf_session__open(struct perf_session *self, bool force)
>>>               self->fd = STDIN_FILENO;
>>>
>>>               if (perf_session__read_header(self, self->fd) < 0)
>>> -                     pr_err("incompatible file format");
>>> +                     pr_err("incompatible file format (rerun with -v to learn more)");
>>>
>>>               return 0;
>>>       }
>>> @@ -56,7 +56,7 @@ static int perf_session__open(struct perf_session *self, bool force)
>>>       }
>>>
>>>       if (perf_session__read_header(self, self->fd) < 0) {
>>> -             pr_err("incompatible file format");
>>> +             pr_err("incompatible file format (rerun with -v to learn more)");
>>>               goto out_close;
>>>       }
>>>