wwbmmm commented on code in PR #3607:
URL: https://github.com/apache/brpc/pull/3607#discussion_r4305356517
##########
src/bvar/variable.cpp:
##########
@@ -689,28 +689,16 @@ int Variable::dump_exposed(Dumper* dumper, const
DumpOptions* poptions) {
// ============= export to files ==============
std::string read_command_name() {
- std::ifstream fin("/proc/self/stat");
- if (!fin.is_open()) {
+ // Use /proc/self/cmdline instead of /proc/self/stat to avoid the 15-byte
+ // TASK_COMM_LEN kernel limit on the comm field. /proc/self/cmdline
+ // contains the full argv[0] which preserves the complete command name.
+ char cmdline[1024];
+ const ssize_t nr = butil::ReadCommandLine(cmdline, sizeof(cmdline), false);
+ if (nr <= 0) {
return std::string();
}
- int pid = 0;
- std::string command_name;
- fin >> pid >> command_name;
- if (!fin.good()) {
- return std::string();
- }
- // Although the man page says the command name is in parenthesis, for
- // safety we normalize the name.
std::string s;
- if (command_name.size() >= 2UL && command_name[0] == '(' &&
- butil::back_char(command_name) == ')') {
- // remove parenthesis.
- to_underscored_name(&s,
- butil::StringPiece(command_name.data() + 1,
- command_name.size() - 2UL));
- } else {
- to_underscored_name(&s, command_name);
- }
+ to_underscored_name(&s, butil::StringPiece(cmdline, nr));
Review Comment:
[replied by brpc-oncall robot] Beyond the path leak Copilot mentioned, this
is a user-visible naming/compatibility change. `comm` used to be a short
executable name, while argv[0] is whatever was used to launch the process;
deployments that start with an absolute or relative path will now produce names
like `_usr_local_bin_myservice` (`to_underscored_name` turns the leading '/'
into '_' and '/' into '_'). That propagates to
`bvar_dump_file`/`bvar_dump_prefix`/`mbvar_*` (`monitor/bvar.<app>.data`,
`<app>` prefix) and to the default `rpc_dump_dir`
(`./rpc_data/rpc_dump/<app>`), so existing monitoring/dump paths and downstream
consumers (e.g. noah) change after upgrade, and the leading '_' is an
additional oddity. Suggest normalizing to the basename of argv[0] (strip
everything up to the last '/') before calling `to_underscored_name`, which
still fixes the 15-char truncation while keeping the previous naming behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
