On Mon, 26 Jun 2017 12:10:04 -0700 Joel Fernandes <joe...@google.com> wrote:
> > That seems Ok with me to do, I think a valid point. > > > > I think that I should do it in the second call to > > tracing_start_tgid_record too then (__ftrace_event_enable_disable) to > > error out if the allocation fails. > > > > While going this code I again, I noticed another potential issue in > > __ftrace_event_enable_disable > > Sorry I hit send too soon! > > I was saying, While going through this code again I noticed another > potential issue in __ftrace_event_enable_disable in existing code: > > Here we do: > ret = call->class->reg(call, TRACE_REG_REGISTER, file); > > But in the error handling path we only do a > tracing_stop_cmdline_record() and not clear the > EVENT_FILE_FL_RECORDED_CMD_BIT flag. Is that not an issue? I am > guessing because enabling of the event itself will fail, that's Ok. > But just to keep it consistent, I am thinking if we should just clear > the bit here. Yeah, we should probably add some better error handling for failed setting of events. But the tgid-map is low hanging fruit to get right ;-) -- Steve