On Fri, Jun 30, 2017 at 2:50 PM, Steven Rostedt <rost...@goodmis.org> wrote:
> On Fri, 30 Jun 2017 11:17:50 -0600
[..]
>>
>> Signed-off-by: Michael Sartain <mikes...@fastmail.com>
>> ---
>>  kernel/trace/trace.c | 60 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index 68c214b..ca84c97 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -4692,6 +4692,7 @@ static const struct file_operations 
>> tracing_readme_fops = {
>>  static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
>>  {
>>       unsigned int *ptr = v;
>> +     long tgid_check = (long) m->private;
>
> I really don't like the subtle use of having m->private != NULL mean
> this is for tgid listing.
>
> In fact, I don't see the purpose of reusing the seq code. The cmdlines
> and tgid map are quite different. Just create its own functions. I
> don't see the benefit of trying to reuse this except for making the
> code more complex.

I agree with Steven, this patch should be rewritten to use separate
functions and not reuse the other ones as its otherwise really
confusing and not good design.

Thanks,
Joel

Reply via email to