Messages in this thread | | | Date | Mon, 30 Jun 2014 19:58:36 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 4/4] tracing/uprobes: Fix the usage of uprobe_buffer_enable() in probe_event_enable() |
| |
On 06/30, Steven Rostedt wrote: > > On Mon, 30 Jun 2014 22:34:09 +0530 > Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote: > > > > + if (ret) > > > + goto err_buffer; > > > > > > + return 0; > > > + > > > + err_buffer: > > > + uprobe_buffer_disable(); > > > + > > > > How about avoiding err_buffer label? > > + if (!ret) > > + return 0; > > > > + uprobe_buffer_disable(); > > + > > > > Oleg, you OK with this update? > > I can kill my tests and restart with this update. Or you can resend this > patch. Or we can just push it as is, and have this be a patch that > get's queued as a cleanup for 3.17?
Well, if you too think that this change can make the code cleaner I should probably make it ;)
But, to me
err = init_1(); if (err) goto err_1;
err = init_2(); if (err) goto err_2;
return 0;
err_2: cleanup_2(); err_1: cleanup_1();
looks better than
err = init_1(); if (err) goto err_1;
err = init_2(); if (!err) return 0;
cleanup_2(); err_1: cleanup_1();
just because the 1st variant is more symmetrical. And in fact it is more flexible, we might add init_3/etc.
But I won't insist, this is subjective. So please let me know if you still think it would be better to add this change, I'll send v2.
Oleg.
| |