Re: [PATCH 7/7] uts namespaces: Implement CLONE_NEWUTS flag

From: Serge E. Hallyn
Date: Tue May 02 2006 - 17:42:25 EST


Quoting Dave Hansen (haveblue@xxxxxxxxxx):
> On Mon, 2006-05-01 at 16:11 -0500, Serge E. Hallyn wrote:
> > Might be worth a separate patch to change over all those helpers in
> > fork.c? (I think they were all brought in along with the sys_unshare
> > syscall)
>
> I'd be a little scared to touch good, working code, but it couldn't hurt
> to see the patch.

Hmm, well the following untested patch was just to see the end results.
Summary: it ends up quite a bit uglier :-(

I think I like it better as is.

thanks,
-serge

Subject: [PATCH] fs/fork.c: unshare cleanup

Switch some of the unshare patch to more kernel conformant style.

Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx>

---

kernel/fork.c | 102 ++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 64 insertions(+), 38 deletions(-)

c30251d8442cfee2ada7dfd6c46159cf44011213
diff --git a/kernel/fork.c b/kernel/fork.c
index d2fa57d..42753a4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1450,86 +1450,92 @@ static int unshare_thread(unsigned long
/*
* Unshare the filesystem structure if it is being shared
*/
-static int unshare_fs(unsigned long unshare_flags, struct fs_struct **new_fsp)
+static struct fs_struct * unshare_fs(unsigned long flags)
{
struct fs_struct *fs = current->fs;
+ struct fs_struct *new_fs = NULL;

- if ((unshare_flags & CLONE_FS) &&
+ if ((flags & CLONE_FS) &&
(fs && atomic_read(&fs->count) > 1)) {
- *new_fsp = __copy_fs_struct(current->fs);
- if (!*new_fsp)
- return -ENOMEM;
+ new_fs = __copy_fs_struct(current->fs);
+ if (!new_fs)
+ new_fs = ERR_PTR(-ENOMEM);
}

- return 0;
+ return new_fs;
}

/*
* Unshare the namespace structure if it is being shared
*/
-static int unshare_namespace(unsigned long unshare_flags, struct namespace **new_nsp, struct fs_struct *new_fs)
+static struct namespace *unshare_namespace(unsigned long flags,
+ struct fs_struct *new_fs)
{
struct namespace *ns = current->namespace;
+ struct namespace *new_ns = NULL;

- if ((unshare_flags & CLONE_NEWNS) &&
+ if ((flags & CLONE_NEWNS) &&
(ns && atomic_read(&ns->count) > 1)) {
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
+ if (!capable(CAP_SYS_ADMIN)) {
+ return ERR_PTR(-EPERM);

- *new_nsp = dup_namespace(current, new_fs ? new_fs : current->fs);
+ *new_ns = dup_namespace(current, new_fs ? new_fs : current->fs);
if (!*new_nsp)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
}

- return 0;
+ return new_ns;
}

/*
* Unsharing of sighand for tasks created with CLONE_SIGHAND is not
* supported yet
*/
-static int unshare_sighand(unsigned long unshare_flags, struct sighand_struct **new_sighp)
+static struct sighand_struct *unshare_sighand(unsigned long flags);
{
struct sighand_struct *sigh = current->sighand;
+ struct sighand_struct *new_sigh = NULL;

- if ((unshare_flags & CLONE_SIGHAND) &&
+ if ((flags & CLONE_SIGHAND) &&
(sigh && atomic_read(&sigh->count) > 1))
- return -EINVAL;
- else
- return 0;
+ return ERR_PTR(-EINVAL);
+
+ return new_sigh;
}

/*
* Unshare vm if it is being shared
*/
-static int unshare_vm(unsigned long unshare_flags, struct mm_struct **new_mmp)
+static mm_struct *unshare_vm(unsigned long flags)
{
struct mm_struct *mm = current->mm;
+ struct mm_struct *new_mm = NULL;

- if ((unshare_flags & CLONE_VM) &&
+ if ((flags & CLONE_VM) &&
(mm && atomic_read(&mm->mm_users) > 1)) {
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}

- return 0;
+ return new_mm;
}

/*
* Unshare file descriptor table if it is being shared
*/
-static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp)
+static struct files_struct *unshare_fd(unsigned long flags)
{
struct files_struct *fd = current->files;
+ struct files_struct *new_fd = NULL;
int error = 0;

- if ((unshare_flags & CLONE_FILES) &&
+ if ((flags & CLONE_FILES) &&
(fd && atomic_read(&fd->count) > 1)) {
- *new_fdp = dup_fd(fd, &error);
- if (!*new_fdp)
- return error;
+ new_fd = dup_fd(fd, &error);
+ if (!new_fd)
+ return ERR_PTR(error);
}

- return 0;
+ return new_fd;
}

/*
@@ -1572,16 +1578,36 @@ asmlinkage long sys_unshare(unsigned lon

if ((err = unshare_thread(unshare_flags)))
goto bad_unshare_out;
- if ((err = unshare_fs(unshare_flags, &new_fs)))
+
+ new_fs = unshare_fs(unshare_flags);
+ if (IS_ERR(new_fs)) {
+ err = PTR_ERR(new_fs);
goto bad_unshare_cleanup_thread;
- if ((err = unshare_namespace(unshare_flags, &new_ns, new_fs)))
+ }
+
+ new_ns = unshare_namespace(unshare_flags, &new_ns);
+ if (IS_ERR(new_ns)) {
+ err = PTR_ERR(new_ns);
goto bad_unshare_cleanup_fs;
- if ((err = unshare_sighand(unshare_flags, &new_sigh)))
+ }
+
+ new_sigh = unshare_sighand(unshare_flags);
+ if (IS_ERR(new_sigh)) {
+ err = PTR_ERR(new_sigh);
goto bad_unshare_cleanup_ns;
- if ((err = unshare_vm(unshare_flags, &new_mm)))
+ }
+ new_mm = unshare_vm(unshare_flags);
+ if (IS_ERR(new_mm)) {
+ err = PTR_ERR(new_mm);
goto bad_unshare_cleanup_sigh;
- if ((err = unshare_fd(unshare_flags, &new_fd)))
+ }
+
+ new_fd = unshare_fd(unshare_flags);
+ if (IS_ERR(new_fd)) {
+ err = PTR_ERR(new_fd);
goto bad_unshare_cleanup_vm;
+ }
+
if ((err = unshare_semundo(unshare_flags, &new_ulist)))
goto bad_unshare_cleanup_fd;

@@ -1626,24 +1652,24 @@ asmlinkage long sys_unshare(unsigned lon
}

bad_unshare_cleanup_fd:
- if (new_fd)
+ if (new_fd && !IS_ERR(new_fd))
put_files_struct(new_fd);

bad_unshare_cleanup_vm:
- if (new_mm)
+ if (new_mm && !IS_ERR(new_mm))
mmput(new_mm);

bad_unshare_cleanup_sigh:
- if (new_sigh)
+ if (new_sigh && !IS_ERR(new_sigh))
if (atomic_dec_and_test(&new_sigh->count))
kmem_cache_free(sighand_cachep, new_sigh);

bad_unshare_cleanup_ns:
- if (new_ns)
+ if (new_ns && !IS_ERR(new_ns))
put_namespace(new_ns);

bad_unshare_cleanup_fs:
- if (new_fs)
+ if (new_fs && !IS_ERR(new_fs)))
put_fs_struct(new_fs);

bad_unshare_cleanup_thread:
--
1.3.0

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/