On Thu, Aug 20, 2015 at 02:52:01PM +0100, Matt Fleming wrote: SNIP
> > The err variable doesn't go down to the add_tracepoint_multi_event() > > call. It actually stops in parse_events_parse() where > > parse_events_add_tracepoint is being called using only the idx part of > > data (util/parse-events.y:389). I think it would be possible to pass > > the whole data variable (struct parse_events_evlist) down those > > variables to still have access to &err, but it would imply quite a lot > > of changes in there. I'm up to it though, if it seems that's the right > > thing to do! What is your take on > > You bring up a good point. Perf would benefit greatly from easy access > to a struct parse_events_error variable, so that it isn't required to > pass it as an argument to every function. > > Now, I know that generally global variables are frowned upon but I > think an exception can be made for error handling, because, assuming > errors are fatal (and if they're not they're called warnings), you > should never have multiple things writing to it at the same time, and > you should only ever execute error paths once you've written to it. > > And if you really, really don't like naked accesses to global > variables you can always use a wrapper function. > > With global access to error data it becomes trivial to improve the > error handling of other functions in a piecemeal way, without > requiring changes to every function in the callstack. No one likes > reviewing large patches ;-) > > I would suggest setting up a global struct parse_events_error object, > and making changes to parse_events_print_error() to handle non-parse > related errors, such as ENOMEM, ENOENT, etc, etc. hum, I haven't digested all of this thread yet, but I happened to actually work on this recently - the tracepoint parsing error propagation ... I did some initial patchset not ready to be posted but working, please check it in: https://git.kernel.org/cgit/linux/kernel/git/jolsa/perf.git/log/?h=perf/tp_5 tracefs and debugfs just give location for accessing tracepoint, the tracing_events_path is currently initialized to one of them we could somehow prettyfy it, like unify all the related interface to make it clear, like the debugfs__strerror_open shouldn't be 'debugfs' specific and take tracefs into account as in my patchset jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
