-
Notifications
You must be signed in to change notification settings - Fork 252
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
Make sure tools never create config.ldb #7551
Conversation
ea1b0d4
to
9a2c874
Compare
Take a note this also enforces check for existance of config.ldb in additional code paths (this is intentional side effect, see a008acc)
8289db3
to
b94f8cf
Compare
Take a note this also enforces check for existance of config.ldb in additional code paths (this is intentional side effect, see a008acc)
b94f8cf
to
12d38cc
Compare
@@ -108,18 +112,14 @@ static errno_t sss_tool_confdb_init(TALLOC_CTX *mem_ctx, | |||
return ret; | |||
} | |||
|
|||
ret = confdb_init(mem_ctx, &confdb, path); | |||
ret = confdb_init(mem_ctx, _confdb, path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related to you commit.
confdb_init()
is assuming its cdb_ctx
parameter is not NULL
. It clearly is not here, but could be in other calls. Shouldn't we also check inside this function that it is not NULL
? Or maybe just move the check to that function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also check inside this function that it is not NULL?
this == sss_tool_confdb_init()
?
But here is the check in place at the beginning (line 97)
Or do I misunderstand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, sorry. this == confdb_init()
.
In other words, sss_tool_confdb_init()
is checking _confdb != NULL
, but other functions are also calling confdb_init()
, and those functions may or may not check that condition. It might be better to move the check inside confdb_init()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, added one more patch.
closer to a place where argument is really used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK. Thanks.
This is an addition to a008acc