Skip to content

Commit

Permalink
fix: Don't recursively call init if log directories can not be created.
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
Janosch Machowinski authored and Janosch Machowinski committed Oct 25, 2023
1 parent 11e9480 commit cf5441c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
8 changes: 7 additions & 1 deletion include/gz/common/Filesystem.hh
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,13 @@ namespace ignition
/// \return True if directory creation was successful, false otherwise.
bool IGNITION_COMMON_VISIBLE createDirectory(const std::string &_path);

/// \brief Create directories for the given path
/// \brief Create directories for the given path errors are printed to the given stream
/// \param[in] _path Path to create directories from
/// \param[in] _errorOut Stream for error output
/// \return true on success
bool IGNITION_COMMON_VISIBLE createDirectories(const std::string &_path, std::ostream &_errorOut);

/// \brief Create directories for the given path errors are printed on ignerr
/// \param[in] _path Path to create directories from
/// \return true on success
bool IGNITION_COMMON_VISIBLE createDirectories(const std::string &_path);
Expand Down
11 changes: 8 additions & 3 deletions src/Console.cc
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ void FileLogger::Init(const std::string &_directory,
{
if (!env(IGN_HOMEDIR, logPath))
{
ignerr << "Missing HOME environment variable."
<< "No log file will be generated.";
std::cerr << "Missing HOME environment variable."
<< "No log file will be generated.";
return;
}
logPath = joinPaths(logPath, _directory);
Expand All @@ -275,7 +275,12 @@ void FileLogger::Init(const std::string &_directory,
auto* buf = dynamic_cast<FileLogger::Buffer*>(this->rdbuf());

// Create the directory if it doesn't exist.
createDirectories(logPath);
if(!createDirectories(logPath, std::cerr))
{
std::cerr << "Failed to generate log directories."
<< "No log file will be generated.";
return;
}

logPath = joinPaths(logPath, _filename);

Expand Down
10 changes: 8 additions & 2 deletions src/Filesystem.cc
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,12 @@ bool common::copyDirectory(const std::string &_existingDirname,

/////////////////////////////////////////////////
bool common::createDirectories(const std::string &_path)
{
return createDirectories(_path, ignerr);
}

/////////////////////////////////////////////////
bool common::createDirectories(const std::string &_path, std::ostream &_errorOut)
{
size_t index = 0;
while (index < _path.size())
Expand All @@ -482,8 +488,8 @@ bool common::createDirectories(const std::string &_path)
if (mkdir(dir.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) != 0)
{
#endif
ignerr << "Failed to create directory [" + dir + "]: "
<< std::strerror(errno) << std::endl;
_errorOut << "Failed to create directory [" + dir + "]: "
<< std::strerror(errno) << std::endl;
return false;
}
}
Expand Down

0 comments on commit cf5441c

Please sign in to comment.