-
Bug
-
Resolution: Unresolved
-
P4
-
20
-
generic
-
linux
The code setting the cgroup path vor cgroups v1 reads like this:
/*
* Set directory to subsystem specific files based
* on the contents of the mountinfo and cgroup files.
*/
void CgroupV1Controller::set_subsystem_path(char *cgroup_path) {
char buf[MAXPATHLEN+1];
if (_root != NULL && cgroup_path != NULL) {
if (strcmp(_root, "/") == 0) {
int buflen;
strncpy(buf, _mount_point, MAXPATHLEN);
buf[MAXPATHLEN-1] = '\0';
if (strcmp(cgroup_path,"/") != 0) {
buflen = strlen(buf);
if ((buflen + strlen(cgroup_path)) > (MAXPATHLEN-1)) {
return;
}
strncat(buf, cgroup_path, MAXPATHLEN-buflen);
buf[MAXPATHLEN-1] = '\0';
}
_path = os::strdup(buf);
} else {
if (strcmp(_root, cgroup_path) == 0) {
strncpy(buf, _mount_point, MAXPATHLEN);
buf[MAXPATHLEN-1] = '\0';
_path = os::strdup(buf);
} else {
char *p = strstr(cgroup_path, _root);
if (p != NULL && p == _root) {
if (strlen(cgroup_path) > strlen(_root)) {
int buflen;
strncpy(buf, _mount_point, MAXPATHLEN);
buf[MAXPATHLEN-1] = '\0';
buflen = strlen(buf);
if ((buflen + strlen(cgroup_path) - strlen(_root)) > (MAXPATHLEN-1)) {
return;
}
strncat(buf, cgroup_path + strlen(_root), MAXPATHLEN-buflen);
buf[MAXPATHLEN-1] = '\0';
_path = os::strdup(buf);
}
}
}
}
}
}
That is for the substring case, we see a 'p = strstr(cgroup_path, _root)' check that is followed by 'if (p != NULL && p == _root)'. We enter this branch only iff _root != '/' AND cgroup_path != _root. So the only way that the pointer points to '_root' (p == _root) is when cgroup_path is the same pointer than _root, but in that case we should have entered the branch where it is checked that cgroup_path == _root. Thus, this is dead code.
I believe the intention of the original author of this code is to check whether there is a substring match of cgroup_path and _root. If cgroup_path starts with _root, the remainder after _root is being appended to the _mount_path. At least that's what the corresponding Java code does.
/*
* Set directory to subsystem specific files based
* on the contents of the mountinfo and cgroup files.
*/
void CgroupV1Controller::set_subsystem_path(char *cgroup_path) {
char buf[MAXPATHLEN+1];
if (_root != NULL && cgroup_path != NULL) {
if (strcmp(_root, "/") == 0) {
int buflen;
strncpy(buf, _mount_point, MAXPATHLEN);
buf[MAXPATHLEN-1] = '\0';
if (strcmp(cgroup_path,"/") != 0) {
buflen = strlen(buf);
if ((buflen + strlen(cgroup_path)) > (MAXPATHLEN-1)) {
return;
}
strncat(buf, cgroup_path, MAXPATHLEN-buflen);
buf[MAXPATHLEN-1] = '\0';
}
_path = os::strdup(buf);
} else {
if (strcmp(_root, cgroup_path) == 0) {
strncpy(buf, _mount_point, MAXPATHLEN);
buf[MAXPATHLEN-1] = '\0';
_path = os::strdup(buf);
} else {
char *p = strstr(cgroup_path, _root);
if (p != NULL && p == _root) {
if (strlen(cgroup_path) > strlen(_root)) {
int buflen;
strncpy(buf, _mount_point, MAXPATHLEN);
buf[MAXPATHLEN-1] = '\0';
buflen = strlen(buf);
if ((buflen + strlen(cgroup_path) - strlen(_root)) > (MAXPATHLEN-1)) {
return;
}
strncat(buf, cgroup_path + strlen(_root), MAXPATHLEN-buflen);
buf[MAXPATHLEN-1] = '\0';
_path = os::strdup(buf);
}
}
}
}
}
}
That is for the substring case, we see a 'p = strstr(cgroup_path, _root)' check that is followed by 'if (p != NULL && p == _root)'. We enter this branch only iff _root != '/' AND cgroup_path != _root. So the only way that the pointer points to '_root' (p == _root) is when cgroup_path is the same pointer than _root, but in that case we should have entered the branch where it is checked that cgroup_path == _root. Thus, this is dead code.
I believe the intention of the original author of this code is to check whether there is a substring match of cgroup_path and _root. If cgroup_path starts with _root, the remainder after _root is being appended to the _mount_path. At least that's what the corresponding Java code does.
- relates to
-
JDK-8343191 Cgroup v1 subsystem fails to set subsystem path
-
- In Progress
-