On Tue, 05.03.13 15:24, Nathaniel Chen ([email protected]) wrote:
Heya,
A few comments on top of what Zbigniew already pointed out.
> + smack = fopen("/sys/fs/smackfs/load2", "w");
Not that it would matter here, but out of principle we generally use "we"
instead of "w"...
> + if (smack == NULL) {
> + m = mount("smackfs", "/smack", "smackfs",
> MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME, "smackfsdef=*");
Please drop this mount() call. Mounts should take place in
mount-setup.c. And the old /smack mount point is not acceptable in
systemd. We should only support the new mount point, and that's it.
> + if (m == -1) {
> + log_error("Smack is not enabled in the kernel. error:
> %s", strerror(errno));
> + return 0;
Suggestion: It's nicer to write %m rather than %s + strerror(errno)...
> + }
> + smack = fopen("/smack/load2", "w");
> + }
> +
> + /* write rules to load2 from every file in the directory */
> + dir = opendir(ACCESSES_D_PATH);
> + if (dir == NULL) {
> + log_error("failed to open Smack policy directory %s, error:
> %s", ACCESSES_D_PATH, strerror(errno));
> + fclose(smack);
> + return -errno;
> + }
> + while ((entry = readdir(dir))) {
> + if (entry->d_name[0] == '.')
> + continue;
Suggestion: FOREACH_DIRENT() is a nice macro for iterating through
directories. It's a fairly new addition, and little used, but it does
make things a bit nicer to read. See src/shared/efivars.c for an example
how to use it.
> +
> + snprintf(pol, PATH_MAX, ACCESSES_D_PATH"%s", entry->d_name);
> + policy = fopen(pol, "r");
"re" is nicer than "r", see above.
> + if (policy == NULL) {
> + log_error("Failed to open Smack policy file %s, error:
> %s", pol, strerror(errno));
> + fclose(smack);
> + closedir(dir);
> + return -errno;
> + }
> + /* load2 write rules in the kernel require a line buffered
> stream */
> + while ((fgets(buf, NAME_MAX, policy))) {
> + fprintf(smack, "%s", buf);
> + fflush(smack);
> + log_info("Smack rule written: %s", buf);
> + }
Consider using the (also new) FOREACH_LINE macro for this iteration. For
an example look at get_process_id() in util.c.
Lennart
--
Lennart Poettering - Red Hat, Inc.
_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
