Skip to contentSkip to conversation

perf machine: Use path__join() to compose a path instead of snprintf(dir, '/', filename)

PATCH Superseded Jisheng Zhang 2 participants 3 messages Sep 12, 2022 – Sep 13, 2022

Subsystems (2)
Stable branch perf events

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jisheng Zhang

author
#0 lore raw

PATCH v2 1/1 perf machine: Use path__join() to compose a path instead of snprintf(dir, '/', filename)

From: Arnaldo Carvalho de Melo <[email protected]>

commit 9d5f0c36438eeae7566ca383b2b673179e3cc613 upstream.

Its more intention revealing, and if we're interested in the odd cases where this may end up truncating we can do debug checks at one centralized place.

Motivation, of all the container builds, fedora rawhide started complaining of:

util/machine.c: In function ‘machine__create_modules’:
util/machine.c:1419:50: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
1419 | snprintf(path, sizeof(path), "%s/%s", dir_name, dent->d_name);
| ^~
In file included from /usr/include/stdio.h:894,
from util/branch.h:9,
from util/callchain.h:8,
from util/machine.c:7:
In function ‘snprintf’,
inlined from ‘maps__set_modules_path_dir’ at util/machine.c:1419:3,
inlined from ‘machine__set_modules_path’ at util/machine.c:1473:9,
inlined from ‘machine__create_modules’ at util/machine.c:1519:7:
/usr/include/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 2 and 4352 bytes into a destination of size 4096

There are other places where we should use path__join(), but lets get rid of this one first.

Cc Adrian Hunter <[email protected]>
Cc Ian Rogers <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Acked-by Ian Rogers <[email protected]>
Link Link: https://lore.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---

Since v1:
- add commit id in upstream.
- add linux-5.15.y, maybe we also need this for other long term stable tree.

 tools/perf/util/machine.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
tools/perf/util/machine.c +2 -1
index 44e40bad0e33..55a041329990 100644
@@ -16,6 +16,7 @@
 #include "map_symbol.h"
 #include "branch.h"
 #include "mem-events.h"
+#include "path.h"
 #include "srcline.h"
 #include "symbol.h"
 #include "sort.h"

@@ -1407,7 +1408,7 @@ static int maps__set_modules_path_dir(struct maps *maps, const char *dir_name, i
 		struct stat st;
 
 		/*sshfs might return bad dent->d_type, so we have to stat*/
-		snprintf(path, sizeof(path), "%s/%s", dir_name, dent->d_name);
+		path__join(path, sizeof(path), dir_name, dent->d_name);
 		if (stat(path, &st))
 			continue;
Signature · 1 line
2.34.1
maintainer
#1 lore raw
Quoted 28 lines · Jisheng Zhang

From: Arnaldo Carvalho de Melo <[email protected]>

commit 9d5f0c36438eeae7566ca383b2b673179e3cc613 upstream.

Its more intention revealing, and if we're interested in the odd cases where this may end up truncating we can do debug checks at one centralized place.

Motivation, of all the container builds, fedora rawhide started complaining of:

util/machine.c: In function ‘machine__create_modules’:
util/machine.c:1419:50: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
1419 | snprintf(path, sizeof(path), "%s/%s", dir_name, dent->d_name);
| ^~
In file included from /usr/include/stdio.h:894,
from util/branch.h:9,
from util/callchain.h:8,
from util/machine.c:7:
In function ‘snprintf’,
inlined from ‘maps__set_modules_path_dir’ at util/machine.c:1419:3,
inlined from ‘machine__set_modules_path’ at util/machine.c:1473:9,
inlined from ‘machine__create_modules’ at util/machine.c:1519:7:
/usr/include/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 2 and 4352 bytes into a destination of size 4096

There are other places where we should use path__join(), but lets get rid of this one first.

Quoted 7 lines
Cc Adrian Hunter <[email protected]>
Cc Ian Rogers <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Acked-by Ian Rogers <[email protected]>
Link Link: https://lore.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

As you are forwarding on this commit, you too need to sign-off on it.

Quoted 6 lines

---

Since v1:
- add commit id in upstream.
- add linux-5.15.y, maybe we also need this for other long term stable tree.

Why is this needed in 5.15?

thanks,

greg k-h

author
#2 lore raw
Quoted 37 lines · Greg Kroah-Hartman
Quoted 28 lines · Jisheng Zhang

From: Arnaldo Carvalho de Melo <[email protected]>

commit 9d5f0c36438eeae7566ca383b2b673179e3cc613 upstream.

Its more intention revealing, and if we're interested in the odd cases where this may end up truncating we can do debug checks at one centralized place.

Motivation, of all the container builds, fedora rawhide started complaining of:

util/machine.c: In function ‘machine__create_modules’:
util/machine.c:1419:50: error: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
1419 | snprintf(path, sizeof(path), "%s/%s", dir_name, dent->d_name);
| ^~
In file included from /usr/include/stdio.h:894,
from util/branch.h:9,
from util/callchain.h:8,
from util/machine.c:7:
In function ‘snprintf’,
inlined from ‘maps__set_modules_path_dir’ at util/machine.c:1419:3,
inlined from ‘machine__set_modules_path’ at util/machine.c:1473:9,
inlined from ‘machine__create_modules’ at util/machine.c:1519:7:
/usr/include/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 2 and 4352 bytes into a destination of size 4096

There are other places where we should use path__join(), but lets get rid of this one first.

Quoted 7 lines
Cc Adrian Hunter <[email protected]>
Cc Ian Rogers <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Acked-by Ian Rogers <[email protected]>
Link Link: https://lore.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
Quoted 1 line

As you are forwarding on this commit, you too need to sign-off on it.

Thanks for the information.

Quoted 9 lines
Quoted 7 lines

---

Since v1:
- add commit id in upstream.
- add linux-5.15.y, maybe we also need this for other long term stable tree.

Why is this needed in 5.15?

Building linux-5.15.y perf with gcc-12 will emit the same error as the commit msg says.

Thanks