Skip to content

Commit 8275637

Browse files
mhiramatrostedt
authored andcommitted
tracing: Adopt __free() and guard() for trace_fprobe.c
Adopt __free() and guard() for trace_fprobe.c to remove gotos. Link: https://lore.kernel.org/173708043449.319651.12242878905778792182.stgit@mhiramat.roam.corp.google.com Signed-off-by: Masami Hiramatsu (Google) <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 4f7caaa commit 8275637

File tree

1 file changed

+58
-66
lines changed

1 file changed

+58
-66
lines changed

kernel/trace/trace_fprobe.c

Lines changed: 58 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,9 @@ static void free_trace_fprobe(struct trace_fprobe *tf)
416416
}
417417
}
418418

419+
/* Since alloc_trace_fprobe() can return error, check the pointer is ERR too. */
420+
DEFINE_FREE(free_trace_fprobe, struct trace_fprobe *, if (!IS_ERR_OR_NULL(_T)) free_trace_fprobe(_T))
421+
419422
/*
420423
* Allocate new trace_probe and initialize it (including fprobe).
421424
*/
@@ -426,7 +429,7 @@ static struct trace_fprobe *alloc_trace_fprobe(const char *group,
426429
struct module *mod,
427430
int nargs, bool is_return)
428431
{
429-
struct trace_fprobe *tf;
432+
struct trace_fprobe *tf __free(free_trace_fprobe) = NULL;
430433
int ret = -ENOMEM;
431434

432435
tf = kzalloc(struct_size(tf, tp.args, nargs), GFP_KERNEL);
@@ -435,7 +438,7 @@ static struct trace_fprobe *alloc_trace_fprobe(const char *group,
435438

436439
tf->symbol = kstrdup(symbol, GFP_KERNEL);
437440
if (!tf->symbol)
438-
goto error;
441+
return ERR_PTR(-ENOMEM);
439442

440443
if (is_return)
441444
tf->fp.exit_handler = fexit_dispatcher;
@@ -447,13 +450,10 @@ static struct trace_fprobe *alloc_trace_fprobe(const char *group,
447450

448451
ret = trace_probe_init(&tf->tp, event, group, false, nargs);
449452
if (ret < 0)
450-
goto error;
453+
return ERR_PTR(ret);
451454

452455
dyn_event_init(&tf->devent, &trace_fprobe_ops);
453-
return tf;
454-
error:
455-
free_trace_fprobe(tf);
456-
return ERR_PTR(ret);
456+
return_ptr(tf);
457457
}
458458

459459
static struct trace_fprobe *find_trace_fprobe(const char *event,
@@ -880,14 +880,12 @@ static int register_trace_fprobe(struct trace_fprobe *tf)
880880
struct trace_fprobe *old_tf;
881881
int ret;
882882

883-
mutex_lock(&event_mutex);
883+
guard(mutex)(&event_mutex);
884884

885885
old_tf = find_trace_fprobe(trace_probe_name(&tf->tp),
886886
trace_probe_group_name(&tf->tp));
887-
if (old_tf) {
888-
ret = append_trace_fprobe(tf, old_tf);
889-
goto end;
890-
}
887+
if (old_tf)
888+
return append_trace_fprobe(tf, old_tf);
891889

892890
/* Register new event */
893891
ret = register_fprobe_event(tf);
@@ -897,7 +895,7 @@ static int register_trace_fprobe(struct trace_fprobe *tf)
897895
trace_probe_log_err(0, EVENT_EXIST);
898896
} else
899897
pr_warn("Failed to register probe event(%d)\n", ret);
900-
goto end;
898+
return ret;
901899
}
902900

903901
/* Register fprobe */
@@ -907,8 +905,6 @@ static int register_trace_fprobe(struct trace_fprobe *tf)
907905
else
908906
dyn_event_add(&tf->devent, trace_probe_event_call(&tf->tp));
909907

910-
end:
911-
mutex_unlock(&event_mutex);
912908
return ret;
913909
}
914910

@@ -1069,7 +1065,10 @@ static int parse_symbol_and_return(int argc, const char *argv[],
10691065
return 0;
10701066
}
10711067

1072-
static int __trace_fprobe_create(int argc, const char *argv[])
1068+
DEFINE_FREE(module_put, struct module *, if (_T) module_put(_T))
1069+
1070+
static int trace_fprobe_create_internal(int argc, const char *argv[],
1071+
struct traceprobe_parse_context *ctx)
10731072
{
10741073
/*
10751074
* Argument syntax:
@@ -1095,23 +1094,20 @@ static int __trace_fprobe_create(int argc, const char *argv[])
10951094
* Type of args:
10961095
* FETCHARG:TYPE : use TYPE instead of unsigned long.
10971096
*/
1098-
struct trace_fprobe *tf = NULL;
1097+
struct trace_fprobe *tf __free(free_trace_fprobe) = NULL;
10991098
int i, new_argc = 0, ret = 0;
11001099
bool is_return = false;
1101-
char *symbol = NULL;
1100+
char *symbol __free(kfree) = NULL;
11021101
const char *event = NULL, *group = FPROBE_EVENT_SYSTEM;
1103-
const char **new_argv = NULL;
1102+
const char **new_argv __free(kfree) = NULL;
11041103
char buf[MAX_EVENT_NAME_LEN];
11051104
char gbuf[MAX_EVENT_NAME_LEN];
11061105
char sbuf[KSYM_NAME_LEN];
11071106
char abuf[MAX_BTF_ARGS_LEN];
1108-
char *dbuf = NULL;
1107+
char *dbuf __free(kfree) = NULL;
11091108
bool is_tracepoint = false;
1110-
struct module *tp_mod = NULL;
1109+
struct module *tp_mod __free(module_put) = NULL;
11111110
struct tracepoint *tpoint = NULL;
1112-
struct traceprobe_parse_context ctx = {
1113-
.flags = TPARG_FL_KERNEL | TPARG_FL_FPROBE,
1114-
};
11151111

11161112
if ((argv[0][0] != 'f' && argv[0][0] != 't') || argc < 2)
11171113
return -ECANCELED;
@@ -1121,13 +1117,11 @@ static int __trace_fprobe_create(int argc, const char *argv[])
11211117
group = TRACEPOINT_EVENT_SYSTEM;
11221118
}
11231119

1124-
trace_probe_log_init("trace_fprobe", argc, argv);
1125-
11261120
if (argv[0][1] != '\0') {
11271121
if (argv[0][1] != ':') {
11281122
trace_probe_log_set_index(0);
11291123
trace_probe_log_err(1, BAD_MAXACT);
1130-
goto parse_error;
1124+
return -EINVAL;
11311125
}
11321126
event = &argv[0][2];
11331127
}
@@ -1137,14 +1131,14 @@ static int __trace_fprobe_create(int argc, const char *argv[])
11371131
/* a symbol(or tracepoint) must be specified */
11381132
ret = parse_symbol_and_return(argc, argv, &symbol, &is_return, is_tracepoint);
11391133
if (ret < 0)
1140-
goto parse_error;
1134+
return -EINVAL;
11411135

11421136
trace_probe_log_set_index(0);
11431137
if (event) {
11441138
ret = traceprobe_parse_event_name(&event, &group, gbuf,
11451139
event - argv[0]);
11461140
if (ret)
1147-
goto parse_error;
1141+
return -EINVAL;
11481142
}
11491143

11501144
if (!event) {
@@ -1160,49 +1154,44 @@ static int __trace_fprobe_create(int argc, const char *argv[])
11601154
}
11611155

11621156
if (is_return)
1163-
ctx.flags |= TPARG_FL_RETURN;
1157+
ctx->flags |= TPARG_FL_RETURN;
11641158
else
1165-
ctx.flags |= TPARG_FL_FENTRY;
1159+
ctx->flags |= TPARG_FL_FENTRY;
11661160

11671161
if (is_tracepoint) {
1168-
ctx.flags |= TPARG_FL_TPOINT;
1162+
ctx->flags |= TPARG_FL_TPOINT;
11691163
tpoint = find_tracepoint(symbol, &tp_mod);
11701164
if (tpoint) {
1171-
ctx.funcname = kallsyms_lookup(
1165+
ctx->funcname = kallsyms_lookup(
11721166
(unsigned long)tpoint->probestub,
11731167
NULL, NULL, NULL, sbuf);
11741168
} else if (IS_ENABLED(CONFIG_MODULES)) {
11751169
/* This *may* be loaded afterwards */
11761170
tpoint = TRACEPOINT_STUB;
1177-
ctx.funcname = symbol;
1171+
ctx->funcname = symbol;
11781172
} else {
11791173
trace_probe_log_set_index(1);
11801174
trace_probe_log_err(0, NO_TRACEPOINT);
1181-
goto parse_error;
1175+
return -EINVAL;
11821176
}
11831177
} else
1184-
ctx.funcname = symbol;
1178+
ctx->funcname = symbol;
11851179

11861180
argc -= 2; argv += 2;
11871181
new_argv = traceprobe_expand_meta_args(argc, argv, &new_argc,
1188-
abuf, MAX_BTF_ARGS_LEN, &ctx);
1189-
if (IS_ERR(new_argv)) {
1190-
ret = PTR_ERR(new_argv);
1191-
new_argv = NULL;
1192-
goto out;
1193-
}
1182+
abuf, MAX_BTF_ARGS_LEN, ctx);
1183+
if (IS_ERR(new_argv))
1184+
return PTR_ERR(new_argv);
11941185
if (new_argv) {
11951186
argc = new_argc;
11961187
argv = new_argv;
11971188
}
1198-
if (argc > MAX_TRACE_ARGS) {
1199-
ret = -E2BIG;
1200-
goto out;
1201-
}
1189+
if (argc > MAX_TRACE_ARGS)
1190+
return -E2BIG;
12021191

12031192
ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
12041193
if (ret)
1205-
goto out;
1194+
return ret;
12061195

12071196
/* setup a probe */
12081197
tf = alloc_trace_fprobe(group, event, symbol, tpoint, tp_mod,
@@ -1211,16 +1200,16 @@ static int __trace_fprobe_create(int argc, const char *argv[])
12111200
ret = PTR_ERR(tf);
12121201
/* This must return -ENOMEM, else there is a bug */
12131202
WARN_ON_ONCE(ret != -ENOMEM);
1214-
goto out; /* We know tf is not allocated */
1203+
return ret;
12151204
}
12161205

12171206
/* parse arguments */
12181207
for (i = 0; i < argc; i++) {
12191208
trace_probe_log_set_index(i + 2);
1220-
ctx.offset = 0;
1221-
ret = traceprobe_parse_probe_arg(&tf->tp, i, argv[i], &ctx);
1209+
ctx->offset = 0;
1210+
ret = traceprobe_parse_probe_arg(&tf->tp, i, argv[i], ctx);
12221211
if (ret)
1223-
goto error; /* This can be -ENOMEM */
1212+
return ret; /* This can be -ENOMEM */
12241213
}
12251214

12261215
if (is_return && tf->tp.entry_arg) {
@@ -1231,7 +1220,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
12311220
ret = traceprobe_set_print_fmt(&tf->tp,
12321221
is_return ? PROBE_PRINT_RETURN : PROBE_PRINT_NORMAL);
12331222
if (ret < 0)
1234-
goto error;
1223+
return ret;
12351224

12361225
ret = register_trace_fprobe(tf);
12371226
if (ret) {
@@ -1242,29 +1231,32 @@ static int __trace_fprobe_create(int argc, const char *argv[])
12421231
trace_probe_log_err(0, BAD_PROBE_ADDR);
12431232
else if (ret != -ENOMEM && ret != -EEXIST)
12441233
trace_probe_log_err(0, FAIL_REG_PROBE);
1245-
goto error;
1234+
return -EINVAL;
12461235
}
12471236

1248-
out:
1249-
if (tp_mod)
1250-
module_put(tp_mod);
1237+
/* 'tf' is successfully registered. To avoid freeing, assign NULL. */
1238+
tf = NULL;
1239+
1240+
return 0;
1241+
}
1242+
1243+
static int trace_fprobe_create_cb(int argc, const char *argv[])
1244+
{
1245+
struct traceprobe_parse_context ctx = {
1246+
.flags = TPARG_FL_KERNEL | TPARG_FL_FPROBE,
1247+
};
1248+
int ret;
1249+
1250+
trace_probe_log_init("trace_fprobe", argc, argv);
1251+
ret = trace_fprobe_create_internal(argc, argv, &ctx);
12511252
traceprobe_finish_parse(&ctx);
12521253
trace_probe_log_clear();
1253-
kfree(new_argv);
1254-
kfree(symbol);
1255-
kfree(dbuf);
12561254
return ret;
1257-
1258-
parse_error:
1259-
ret = -EINVAL;
1260-
error:
1261-
free_trace_fprobe(tf);
1262-
goto out;
12631255
}
12641256

12651257
static int trace_fprobe_create(const char *raw_command)
12661258
{
1267-
return trace_probe_create(raw_command, __trace_fprobe_create);
1259+
return trace_probe_create(raw_command, trace_fprobe_create_cb);
12681260
}
12691261

12701262
static int trace_fprobe_release(struct dyn_event *ev)

0 commit comments

Comments
 (0)