On Thu, 30.09.10 18:10, [email protected] ([email protected]) wrote:
A last round of comments:
> +int umount_init(void) {
> + LIST_HEAD(MountPoint, mp_list_head);
> + int r;
> + bool changed;
This isn't really an "initialization", is it? I'd prefer the name umount_all().
> +
> + /* Preventing that we won't block umounts */
> + if ((chdir("/")) == -1)
> + return -1;
In systemd we use negative errno error codes, like the kernel (also see
CODING_STYLE). In this case:
if (chdir("/") < 0)
return -errno;
> +
> + LIST_HEAD_INIT(MountPoint, mp_list_head);
> + if (mount_points_list_get(&mp_list_head) < 0)
> + return -1;
Same thing here.
if ((r = mount_points_list_get(&mp_list_head)) < 0)
return r;
(Assuming m_p_l_g() returns an errno error code, too)
> +
> + for (changed = false; changed;)
> + mount_points_list_umount(&mp_list_head, &changed);
> +
> + for (changed = false; changed;)
> + r = mount_points_list_remount_read_only(&mp_list_head,
> &changed);
> +
> + mount_points_list_free(&mp_list_head);
> +
> + return r;
I think it would make sense to return the last error we
encounter. Currently if a later mount_points_list_remount_read_only()
succeeds an earlier error is ignored.
Lennart
--
Lennart Poettering - Red Hat, Inc.
_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
