On Fri, 11.07.14 15:05, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> This avoids errors like this, when the paths are already there with the
> correct permissions and owner:
>
> chmod(/var/spool) failed: Read-only file system
> ---
> src/tmpfiles/tmpfiles.c | 36 +++++++++++++++++++++---------------
> 1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> index 68cfa55..4f41f28 100644
> --- a/src/tmpfiles/tmpfiles.c
> +++ b/src/tmpfiles/tmpfiles.c
> @@ -453,35 +453,41 @@ finish:
> }
>
> static int item_set_perms(Item *i, const char *path) {
> + struct stat st;
> + mode_t old_m = ~0;
> + uid_t old_uid = -1;
> + gid_t old_gid = -1;
I'd prefer an explicit cast here, to "(uid_t) -1", since uid_t is
actually unsigned, and we should clarify what we are doing here.
> +
> assert(i);
> assert(path);
>
> + if (stat(path, &st) >= 0) {
> + old_m = st.st_mode & 07777;
> + old_uid = st.st_uid;
> + old_gid = st.st_gid;
> + }
Hmm, why copy the fields out here? Can't we just initialize them in the
struct directly to something useful, and just always use the structure
directly?
> /* not using i->path directly because it may be a glob */
> if (i->mode_set) {
> mode_t m = i->mode;
>
> - if (i->mask_perms) {
> - struct stat st;
> -
> - if (stat(path, &st) >= 0) {
> - if (!(st.st_mode & 0111))
> - m &= ~0111;
> - if (!(st.st_mode & 0222))
> - m &= ~0222;
> - if (!(st.st_mode & 0444))
> - m &= ~0444;
> - if (!S_ISDIR(st.st_mode))
> - m &= ~07000; /* remove
> sticky/sgid/suid bit, unless directory */
> - }
> + if (i->mask_perms && old_m != ~0) {
So you are using "old_m != ~0" as check whether the stat() was
successfuly? I'd really prefer if we could make this explicit. Just add
a bool called "st_valid" or so, and check that here. That's certainly
easier to read.
Otherwise looks good!
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
