On Mon, Nov 4, 2013 at 5:06 PM, Lennart Poettering <lenn...@poettering.net> wrote:
> Sorry, I don't understand what this patch is doing. Please explain in a > commit message! The file format should also be documented in the code itself, if not done by selinx, then we need to add the link to the doc. >> --- >> src/core/selinux-access.c | 59 >> ++++++++++++++++++++++++++++++++++++++++++++++- >> src/core/selinux-access.h | 15 +++++++++--- >> 2 files changed, 70 insertions(+), 4 deletions(-) >> >> diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c >> index 0a3ee18..5908a79 100644 >> --- a/src/core/selinux-access.c >> +++ b/src/core/selinux-access.c >> @@ -333,6 +333,50 @@ static int get_calling_context( >> return 0; >> } >> >> +static int load_context( >> + const char *path, >> + const char *name, >> + char **context) { >> + >> + int r = 0; >> + char l[LINE_MAX]; >> + char *p; >> + FILE *fd; >> + >> + fd = fopen(path, "r"); > > Please always use "re" rather than "r". Not that it would matter much > here, but Please always do. > >> + if (!fd) >> + return -EEXIST; >> + >> + if (!fgets(l, sizeof(l), fd)) { > > Hmm, isn't this a job for read_one_line_file()? > >> + if (feof(fd)) { >> + r = -EINVAL; >> + goto finish; >> + } >> + >> + log_error("Failed to read context file '%s': %m", path); >> + r = -errno; >> + goto finish; >> + } >> + >> + p = strtok (l, "="); > > strtok() is evil, as it keeps internal state, please don't use it. Also > one space too much... > >> + if (p && streq(p, name)) { >> + p = strtok (NULL, "="); >> + if (!p) { >> + r = -EINVAL; >> + goto finish; >> + } >> + >> + *context = strdup(p); > > Missing OOM check. > >> + } else >> + r = -EINVAL; >> + >> +finish: >> + if (fd) >> + fclose(fd); > > Please use _cleanup_close_ for cases like this. It should probably just use: parse_env_file() it does all that. >> + } else if (class == CLASS_TRANSIENT) { >> + const char *context_path = >> selinux_systemd_contexts_path(); > > Where is selinux_systemd_contexts_path() define? I cant find it in this > patch and neither in the sources? What am I missing? http://people.fedoraproject.org/~dwalsh/SELinux/Patches/0008-Add-selinux_systemd_contexts_path.patch >> + if (_u->source_path || _u->fragment_path) \ >> + _r = selinux_access_check(_c, _m, CLASS_SERVICE, >> _u->source_path ?: _u->fragment_path, (permission), &_error); \ >> + else \ >> + _r = selinux_access_check(_c, _m, CLASS_TRANSIENT, >> _u->id, (permission), &_error); \ > > You can recognize transient units via the transient bool on the Unit > structure. Kay _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel