Skip to content

Commit 2de1bd9

Browse files
Fix segfault in case of no write access to log dir (#546)
* fix: Don't recursively call init if log directories can not be created. If there is no permission to create the log directories, the FileLogger will call itself recutsive, as is used ignerr for logging the error, which will call FileLogger::Init again. This commit fixed this, by using std::cerr for the log output in FileLogger::Init. Signed-off-by: Janosch Machowinski <[email protected]> Co-authored-by: Ian Chen <[email protected]>
1 parent 11e9480 commit 2de1bd9

File tree

4 files changed

+24
-7
lines changed

4 files changed

+24
-7
lines changed

include/gz/common/Filesystem.hh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,13 @@ namespace ignition
6363
/// \return True if directory creation was successful, false otherwise.
6464
bool IGNITION_COMMON_VISIBLE createDirectory(const std::string &_path);
6565

66-
/// \brief Create directories for the given path
66+
/// \brief Create directories for the given path errors are printed to the given stream
67+
/// \param[in] _path Path to create directories from
68+
/// \param[in] _errorOut Stream for error output
69+
/// \return true on success
70+
bool IGNITION_COMMON_VISIBLE createDirectories(const std::string &_path, std::ostream &_errorOut);
71+
72+
/// \brief Create directories for the given path errors are printed on ignerr
6773
/// \param[in] _path Path to create directories from
6874
/// \return true on success
6975
bool IGNITION_COMMON_VISIBLE createDirectories(const std::string &_path);

src/Console.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,8 @@ void FileLogger::Init(const std::string &_directory,
261261
{
262262
if (!env(IGN_HOMEDIR, logPath))
263263
{
264-
ignerr << "Missing HOME environment variable."
265-
<< "No log file will be generated.";
264+
std::cerr << "Missing HOME environment variable. "
265+
<< "No log file will be generated.";
266266
return;
267267
}
268268
logPath = joinPaths(logPath, _directory);
@@ -275,7 +275,12 @@ void FileLogger::Init(const std::string &_directory,
275275
auto* buf = dynamic_cast<FileLogger::Buffer*>(this->rdbuf());
276276

277277
// Create the directory if it doesn't exist.
278-
createDirectories(logPath);
278+
if(!createDirectories(logPath, std::cerr))
279+
{
280+
std::cerr << "Failed to generate log directories. "
281+
<< "No log file will be generated.";
282+
return;
283+
}
279284

280285
logPath = joinPaths(logPath, _filename);
281286

src/EnumIface_TEST.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ IGN_ENUM(myTypeIface, MyType, MY_TYPE_BEGIN, MY_TYPE_END,
4141
/////////////////////////////////////////////////
4242
TEST_F(EnumIfaceTest, StringCoversion)
4343
{
44-
MyType type;
44+
MyType type = MyType::TYPE2;
4545

4646
// Set value from string
4747
myTypeIface.Set(type, "TYPE1");

src/Filesystem.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,12 @@ bool common::copyDirectory(const std::string &_existingDirname,
465465

466466
/////////////////////////////////////////////////
467467
bool common::createDirectories(const std::string &_path)
468+
{
469+
return createDirectories(_path, ignerr);
470+
}
471+
472+
/////////////////////////////////////////////////
473+
bool common::createDirectories(const std::string &_path, std::ostream &_errorOut)
468474
{
469475
size_t index = 0;
470476
while (index < _path.size())
@@ -482,8 +488,8 @@ bool common::createDirectories(const std::string &_path)
482488
if (mkdir(dir.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) != 0)
483489
{
484490
#endif
485-
ignerr << "Failed to create directory [" + dir + "]: "
486-
<< std::strerror(errno) << std::endl;
491+
_errorOut << "Failed to create directory [" + dir + "]: "
492+
<< std::strerror(errno) << std::endl;
487493
return false;
488494
}
489495
}

0 commit comments

Comments
 (0)