Messages in this thread | | | Date | Tue, 07 Feb 2012 09:41:20 -0700 | From | David Ahern <> | Subject | Re: [PATCH v5 16/18] perf: enable reading of perf.data files from different ABI rev |
| |
On 02/07/2012 08:50 AM, Stephane Eranian wrote: >>> + if (sz == 0) { >>> + /* assume ABI0 */ >>> + sz = PERF_ATTR_SIZE_VER0; >> >> Shouldn't this be a failure? ie., problem with the file (or the >> swapping) since size can't be 0 >> > size can be zero. In which case, it means ABI0 version. > See kernel/event/core.c:perf_copy_attr().
ok
> > >> And then for the following why not restrict sz to known, expected sizes >> -- using the PERF_ATTR_SIZE_VER defines introduced in patch 15? >> > Well, the current code solves the problem once and for all. Old tools > can still read new files and vice-versa. If you think that's a problem I > can simply bail out if sz > our_sz.
My sensitivity on this is when endianness is broken it is a nightmare to find. You end up lacing the code with printfs trying to find which size field is going off the charts making the parsing of the file fail - or worse the sizes are slightly off and you get non-sense out.
New commands should be able to read old files; old commands reading new files is a bit of a stretch in that the code has to be future-proofed. It seems like a reasonable requirement for data files to be examined by a command of the same vintage or newer as the command that wrote the file.
David
| |