F/file perms extract from cfg #984
Conversation
This patch is in preperation of using FilePermOptions to store file/dir uid/gid/perm triplets, instead of duplicating most of the data struct within GlobalConfig. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
This patch is still a preparation, reworks the GlobalConfig methods to work with data within FilePermOptions. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
Instead of using the GlobalConfig specific functions, use the same rules to parse the file uid/gid/perm triplet. NOTE: that directory specific rules are not migrated yet, as those were previously not supported by per-destination settings. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
This patch gets rid off the GlobalConfig specific uid/gid/perm functions in favour of FilePermOptions. There were separate rules for file and directory specific uid/gid/perm settings, but in reality we always used the broader one. So this patch merges the two. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
|
Travis seems to have acked it, adding the needs-review label. I have some further work queued, which depends on this one, so a review would be appreciated. |
|
Hi,
|
|
Sure, we can, and its better that way.
|
| @@ -45,7 +45,8 @@ void file_perm_options_set_dir_gid(FilePermOptions *s, const gchar *dir_gid); | |||
| void file_perm_options_set_dir_perm(FilePermOptions *s, gint dir_perm); | |||
|
|
|||
| void file_perm_options_defaults(FilePermOptions *self); | |||
| void file_perm_options_init(FilePermOptions *self, GlobalConfig *cfg); | |||
| void file_perm_options_global_defaults(FilePermOptions *self); | |||
| void file_perm_options_init(FilePermOptions *self, FilePermOptions *global_options); | |||
There was a problem hiding this comment.
I would add a const here for the argument and at some other places as well.
|
If I want to nitpick, I'd generally keep the first line of each commit message at most 72 characters long, otherwise GitHub will break it intrusively. gitk still shows it nicely, but anyway I just wanted to let you know for consideration. Most projects follow a 50/72 commit message rule, but they generally recommend hard breaking on 72 in any case, so I don't think that 72/72 is an unreasonable rule to follow. |
|
Other than the message subject improvement and the recommendation that we should strive to use (struct) constness for new codes, I see no issue with this PR. 👍 |
bazsi
force-pushed
the
f/file-perms-extract-from-cfg
branch
from
4eb62ce
to
ac5e4aa
Compare
Previously the extremal value -2 was used to indicate that we don't want global values to be inherited, and this extremal value was even exposed to call sites. Avoid that by definining specialization functions for this case. Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
afunix does not want the global {file,dir}_uid/gid/perm values to
be inherited, but still the *Options convention in syslog-ng dictates that
FilePermOptions should be initialized, otherwise we would be using
an uninitialized object.
This code adds two specialized init cases, one that inherits the global
options file_perm_options_inherit_from() and one that does the initialization
but explicitly doesn't inherit from global options.
afunix calls the latter, whereby all other call-sites use the former.
Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
bazsi
force-pushed
the
f/file-perms-extract-from-cfg
branch
from
ac5e4aa
to
b86ebd8
Compare
|
I think I have now addressed all review comments, clarified commit message, so this should be ready for merging, once travis acks that. |
|
Hi, thanks! |
This series extracts a few members from GlobalConfig and uses the FilePermOptions structure instead.
Again a step in decoupling GlobalConfig dependency from various parts of syslog-ng.
Since testing is lacking in the area, I split up the series into really small baby steps that should be possible to validate by reviewing it.
Let's see what travis thinks about it :)