[jira] [Commented] (MESOS-2412) Potential memleak(s) in stout/os.hpp
[
https://issues.apache.org/jira/browse/MESOS-2412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14343054#comment-14343054
]
Till Toenshoff commented on MESOS-2412:
---
commit 4337411d3db1207170577e6a52ebf410d65c851e
Author: Joerg Schad jo...@mesosphere.io
Date: Thu Feb 26 17:39:49 2015 -0800
Fix potential memleak is stout/os.hpp.
Review: https://reviews.apache.org/r/31494
Potential memleak(s) in stout/os.hpp
Key: MESOS-2412
URL: https://issues.apache.org/jira/browse/MESOS-2412
Project: Mesos
Issue Type: Bug
Components: stout
Reporter: Joerg Schad
Assignee: Joerg Schad
Labels: coverity, twitter
Coverity picked up this potential memleak in os.hpp where we do not delete
buffer in the else case. The exact same pattern occurs in getuid(const
Optionstd::string user = None()).
The corresponding CID 1230371 and 1230371.
{code}
inline Resultgid_t getgid(const Optionstd::string user = None())
...
while (true) {
char* buffer = new char[size];
if (getpwnam_r(user.get().c_str(), passwd, buffer, size, result) == 0) {
...
delete[] buffer;
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
delete[] buffer;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-2412) Potential memleak(s) in stout/os.hpp
[
https://issues.apache.org/jira/browse/MESOS-2412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14338796#comment-14338796
]
Dominic Hamon commented on MESOS-2412:
--
https://reviews.apache.org/r/31489/
Potential memleak(s) in stout/os.hpp
Key: MESOS-2412
URL: https://issues.apache.org/jira/browse/MESOS-2412
Project: Mesos
Issue Type: Bug
Components: stout
Reporter: Joerg Schad
Assignee: Dominic Hamon
Labels: coverity, twitter
Coverity picked up this potential memleak in os.hpp where we do not delete
buffer in the else case. The exact same pattern occurs in getuid(const
Optionstd::string user = None()).
The corresponding CID 1230371 and 1230371.
{code}
inline Resultgid_t getgid(const Optionstd::string user = None())
...
while (true) {
char* buffer = new char[size];
if (getpwnam_r(user.get().c_str(), passwd, buffer, size, result) == 0) {
...
delete[] buffer;
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
delete[] buffer;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-2412) Potential memleak(s) in stout/os.hpp
[
https://issues.apache.org/jira/browse/MESOS-2412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14339165#comment-14339165
]
Joerg Schad commented on MESOS-2412:
Simply introducing the delete[] statements. See discussion in above review:
https://reviews.apache.org/r/31494/
[~dhamon]: Sorry, I don't mean to interfere with your patch, but had that
actually prepared the patch this morning after creating this Jira.
Potential memleak(s) in stout/os.hpp
Key: MESOS-2412
URL: https://issues.apache.org/jira/browse/MESOS-2412
Project: Mesos
Issue Type: Bug
Components: stout
Reporter: Joerg Schad
Assignee: Dominic Hamon
Labels: coverity, twitter
Coverity picked up this potential memleak in os.hpp where we do not delete
buffer in the else case. The exact same pattern occurs in getuid(const
Optionstd::string user = None()).
The corresponding CID 1230371 and 1230371.
{code}
inline Resultgid_t getgid(const Optionstd::string user = None())
...
while (true) {
char* buffer = new char[size];
if (getpwnam_r(user.get().c_str(), passwd, buffer, size, result) == 0) {
...
delete[] buffer;
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
delete[] buffer;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-2412) Potential memleak(s) in stout/os.hpp
[
https://issues.apache.org/jira/browse/MESOS-2412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14339185#comment-14339185
]
Alexander Rojas commented on MESOS-2412:
I think we rather go with [~dhamon] patch. It's safer and as shown is easy to
forget to add a {delete} if another return statement is added.
Potential memleak(s) in stout/os.hpp
Key: MESOS-2412
URL: https://issues.apache.org/jira/browse/MESOS-2412
Project: Mesos
Issue Type: Bug
Components: stout
Reporter: Joerg Schad
Assignee: Dominic Hamon
Labels: coverity, twitter
Coverity picked up this potential memleak in os.hpp where we do not delete
buffer in the else case. The exact same pattern occurs in getuid(const
Optionstd::string user = None()).
The corresponding CID 1230371 and 1230371.
{code}
inline Resultgid_t getgid(const Optionstd::string user = None())
...
while (true) {
char* buffer = new char[size];
if (getpwnam_r(user.get().c_str(), passwd, buffer, size, result) == 0) {
...
delete[] buffer;
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
delete[] buffer;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-2412) Potential memleak(s) in stout/os.hpp
[
https://issues.apache.org/jira/browse/MESOS-2412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14339199#comment-14339199
]
Joerg Schad commented on MESOS-2412:
I agree that removing raw pointers is the safer solution, but I feel this
change (i.e. remove raw pointer from ...) should be addressed in a separate
Jira with the proper discussion.
Potential memleak(s) in stout/os.hpp
Key: MESOS-2412
URL: https://issues.apache.org/jira/browse/MESOS-2412
Project: Mesos
Issue Type: Bug
Components: stout
Reporter: Joerg Schad
Assignee: Dominic Hamon
Labels: coverity, twitter
Coverity picked up this potential memleak in os.hpp where we do not delete
buffer in the else case. The exact same pattern occurs in getuid(const
Optionstd::string user = None()).
The corresponding CID 1230371 and 1230371.
{code}
inline Resultgid_t getgid(const Optionstd::string user = None())
...
while (true) {
char* buffer = new char[size];
if (getpwnam_r(user.get().c_str(), passwd, buffer, size, result) == 0) {
...
delete[] buffer;
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
delete[] buffer;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-2412) Potential memleak(s) in stout/os.hpp
[
https://issues.apache.org/jira/browse/MESOS-2412?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14338166#comment-14338166
]
Alexander Rojas commented on MESOS-2412:
This is the kind of cases why raw pointers should be avoided. In these case, I
would even suggest using something like:
{code}
while (true) {
std::vectorchar buffer(size);
if (getpwnam_r(user.get().c_str(), passwd, buffer[0], buffer.size(),
result) == 0) {
...
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
}
{code}
As you see, no need to care of the buffer lifecycle.
Potential memleak(s) in stout/os.hpp
Key: MESOS-2412
URL: https://issues.apache.org/jira/browse/MESOS-2412
Project: Mesos
Issue Type: Bug
Components: stout
Reporter: Joerg Schad
Labels: coverity
Coverity picked up this potential memleak in os.hpp where we do not delete
buffer in the else case. The exact same pattern occurs in getuid(const
Optionstd::string user = None()).
The corresponding CID 1230371 and 1230371.
{code}
inline Resultgid_t getgid(const Optionstd::string user = None())
...
while (true) {
char* buffer = new char[size];
if (getpwnam_r(user.get().c_str(), passwd, buffer, size, result) == 0) {
...
delete[] buffer;
return gid;
} else {
// RHEL7 (and possibly other systems) will return non-zero and
// set one of the following errors for The given name or uid
// was not found. See 'man getpwnam_r'. We only check for the
// errors explicitly listed, and do not consider the ellipsis.
if (errno == ENOENT ||
errno == ESRCH ||
errno == EBADF ||
errno == EPERM) {
return None();
// HERE WE DO NOT DELETE BUFFER.
}
...
// getpwnam_r set ERANGE so try again with a larger buffer.
size *= 2;
delete[] buffer;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
