Skip to contentSkip to conversation

perf probe: Clean up probe_point_lazy_walker() return value

tip:perf/core Peter Zijlstra 3 participants 3 messages Mar 16, 2011 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

tools/perf/util/probe-finder.c static int probe_point_lazy_walker(const char *fname, int lineno, · line 1332

Quoted 11 lines · tip-bot for Ingo Molnar

perf probe: Clean up probe_point_lazy_walker() return value

tools/perf/util/probe-finder.c +1 -1
@@ -1328,7 +1328,7 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
 	 * Continue if no error, because the lazy pattern will match
 	 * to other lines
 	 */
-	return ret < 0 ?: 0;
+	return ret < 0 ? ret : 0;
 }

It is a slight change in semantics though, the return value will now be negative instead of 1. If its used as a boolean that's fine, but still.

I'd have changed it to:

return ret < 0;

Which is identical to the previous statement.

#2 lore raw
Quoted 17 lines · Peter Zijlstra

tools/perf/util/probe-finder.c static int probe_point_lazy_walker(const char *fname, int lineno, · line 1332

Quoted 11 lines · tip-bot for Ingo Molnar

perf probe: Clean up probe_point_lazy_walker() return value

tools/perf/util/probe-finder.c +1 -1
@@ -1328,7 +1328,7 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
 	 * Continue if no error, because the lazy pattern will match
 	 * to other lines
 	 */
-	return ret < 0 ?: 0;
+	return ret < 0 ? ret : 0;
 }

It is a slight change in semantics though, the return value will now be negative instead of 1. If its used as a boolean that's fine, but still.

Quoted 5 lines

I'd have changed it to:

return ret < 0;

Which is identical to the previous statement.

Sorry this was my mistake, that statement actually aims to what Ingo has done.

If probe_point_lazy_walker() returns non-zero value, die_walk_lines() returns immediately, because a positive return value means the handler finds line and wanna stop walking on the lines, and a negative return value means some errors occurred in the handler and must stop.

However, probe_point_lazy_walker() has to continue to walk, because given lazy-pattern can match several lines. Thus, this must return 0 if ret is a positive value, and return an error code if ret is a negative value.

Thank you,

Signature · 4 lines
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: [email protected]
#3 lore raw
In reply to Peter Zijlstra

Em Wed, Mar 16, 2011 at 08:59:17AM +0100, Peter Zijlstra escreveu:

Quoted 17 lines · tip-bot for Ingo Molnar

tools/perf/util/probe-finder.c static int probe_point_lazy_walker(const char *fname, int lineno, · line 1332

Quoted 12 lines

perf probe: Clean up probe_point_lazy_walker() return value

tools/perf/util/probe-finder.c +1 -1
@@ -1328,7 +1328,7 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
 	 * Continue if no error, because the lazy pattern will match
 	 * to other lines
 	 */
-	return ret < 0 ?: 0;
+	return ret < 0 ? ret : 0;
 }

It is a slight change in semantics though, the return value will now be negative instead of 1. If its used as a boolean that's fine, but still.

Quoted 5 lines

I'd have changed it to:

return ret < 0;

Which is identical to the previous statement.

Looks similar to the problem fixed in:

fbee632d0ca9f4073a3fefb9a843eac8af036b0f

for another function, I bet the intent in both cases was to return ret, i.e. the negative value.

- Arnaldo