Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfault in case of no write access to log dir #546

Merged
merged 4 commits into from
Nov 13, 2023

Conversation

jmachowinski
Copy link
Contributor

🦟 Bug fix

Fixes a Segfault when building Ign common3 with pbuilder

Summary

If there is no write access to the log directory, the logger will call itself recusively in an infinite loop and segfault.
Note, this bug is also present in the latest version of ign common.

Janosch Machowinski added 2 commits October 25, 2023 11:05
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]>
Signed-off-by: Janosch Machowinski <[email protected]>
@jmachowinski jmachowinski requested a review from marcoag as a code owner October 25, 2023 09:11
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Oct 25, 2023
@jmachowinski jmachowinski changed the title Ign common3 Fix segfault in case of no write access to log dir Oct 25, 2023
@mjcarroll
Copy link
Contributor

Note, this bug is also present in the latest version of ign common.

once merged, this will be forward ported across versions.

src/Console.cc Outdated Show resolved Hide resolved
src/Console.cc Outdated Show resolved Hide resolved
jmachowinski and others added 2 commits October 27, 2023 15:53
Co-authored-by: Ian Chen <[email protected]>
Signed-off-by: jmachowinski <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Signed-off-by: jmachowinski <[email protected]>
@jmachowinski
Copy link
Contributor Author

ping ?

@@ -41,7 +41,7 @@ IGN_ENUM(myTypeIface, MyType, MY_TYPE_BEGIN, MY_TYPE_END,
/////////////////////////////////////////////////
TEST_F(EnumIfaceTest, StringCoversion)
{
MyType type;
MyType type = MyType::TYPE2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change related to the rest of the pull request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is a separate commit fixing a compile warning.

Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small question, overall LGTM

@mjcarroll mjcarroll merged commit 2de1bd9 into gazebosim:ign-common3 Nov 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants