On 05/19/2015 09:31 PM, Yuan Sun wrote:
> Signed-off-by: Yuan Sun <sunyu...@huawei.com>
> ---
> testcases/kernel/containers/userns/Makefile | 28 ++++++
> testcases/kernel/containers/userns/README | 22 +++++
> testcases/kernel/containers/userns/userns01.c | 104
> +++++++++++++++++++++
> testcases/kernel/containers/userns/userns_helper.h | 37 ++++++++
> 4 files changed, 191 insertions(+)
> create mode 100644 testcases/kernel/containers/userns/Makefile
> create mode 100644 testcases/kernel/containers/userns/README
> create mode 100644 testcases/kernel/containers/userns/userns01.c
> create mode 100644 testcases/kernel/containers/userns/userns_helper.h
Hi,
I suggest using checkpatch.pl, which can help you identify places
where you (presumably) copied also style issues from original code:
line going over 80 characters
mixing spaces and tabs
whitespace at the end of lines
>
> diff --git a/testcases/kernel/containers/userns/Makefile
> b/testcases/kernel/containers/userns/Makefile
> new file mode 100644
> index 0000000..6dfe694
> --- /dev/null
> +++ b/testcases/kernel/containers/userns/Makefile
> @@ -0,0 +1,28 @@
> +###############################################################################
> +#
> ##
> +# Copyright (c) International Business Machines Corp., 2007
> ##
You can use your name/company or "Linux Test Project" with current year
> +#
> ##
> +# This program is free software; you can redistribute it and#or modify
> ##
> +# it under the terms of the GNU General Public License as published by
> ##
> +# the Free Software Foundation; either version 2 of the License, or
> ##
> +# (at your option) any later version.
> ##
> +#
> ##
> +# This program is distributed in the hope that it will be useful, but
> ##
> +# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> ##
> +# or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> ##
> +# for more details.
> ##
> +#
> ##
> +# You should have received a copy of the GNU General Public License
> ##
> +# along with this program; if not, write to the Free Software
> ##
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA ##
Line above is clearly more than 80 characters. You can drop the part about
the address of FSF.
> +#
> ##
> +###############################################################################
> +
> +top_srcdir ?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(abs_srcdir)/../Makefile.inc
> +
> +LDLIBS := -lpthread -lrt -lclone -lltp
Is -lpthread and -lrt needed here?
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/containers/userns/README
> b/testcases/kernel/containers/userns/README
> new file mode 100644
> index 0000000..8011cff
> --- /dev/null
> +++ b/testcases/kernel/containers/userns/README
This README seems redundant, there is description in userns01.c
> @@ -0,0 +1,22 @@
> +################################################################################
> +##
> ##
> +## Copyright (c) International Business Machines Corp., 2007
> ##
> +##
> ##
> +## This program is free software; you can redistribute it and#or modify
> ##
> +## it under the terms of the GNU General Public License as published by
> ##
> +## the Free Software Foundation; either version 2 of the License, or
> ##
> +## (at your option) any later version.
> ##
> +##
> ##
> +## This program is distributed in the hope that it will be useful, but
> ##
> +## WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY ##
> +## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> ##
> +## for more details.
> ##
> +##
> ##
> +## You should have received a copy of the GNU General Public License
> ##
> +## along with this program; if not, write to the Free Software
> ##
> +## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA ##
> +##
> ##
> +################################################################################
> +
> +USERNS testcases Overview:
> +User namespaces allow per-namespace mappings of user and group IDs. This
> means that a process's user and group IDs inside a user namespace can be
> different from its IDs outside of the namespace.
> diff --git a/testcases/kernel/containers/userns/userns01.c
> b/testcases/kernel/containers/userns/userns01.c
> new file mode 100644
> index 0000000..ca516e7
> --- /dev/null
> +++ b/testcases/kernel/containers/userns/userns01.c
> @@ -0,0 +1,104 @@
> +/*
> +* Copyright (c) International Business Machines Corp., 2007
> +* This program is free software; you can redistribute it and/or modify
> +* it under the terms of the GNU General Public License as published by
> +* the Free Software Foundation; either version 2 of the License, or
> +* (at your option) any later version.
> +* This program is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> +* the GNU General Public License for more details.
> +* You should have received a copy of the GNU General Public License
> +* along with this program; if not, write to the Free Software
> +* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
This GPL block looks a bit mangled.
> +*
> +***************************************************************************
> +
> +* File: userns01.c
The above comments like what file we are in and "*" separator lines are useless.
> +*
> +* Description:
> +* The userns01.c testcase builds into the ltp framework to verify
> +* the basic functionality of USER Namespace.
The above description is too generic and doesn't really help to explain what
is this test about.
> +*
> +* Verify that:
> +* If a user ID has no mapping inside the namespace, user ID and group
> +* ID will be the value defined in the file /proc/sys/kernel/overflowuid,
> 65534.
This is good, that gives reader some clue what this test is about.
> +*
> +* Total Tests:
> +*
> +* Test Name: userns01
> +*
> +* History:
> +*
> +* FLAG DATE NAME DESCRIPTION
> +* 16/05/15 Yuan Sun <sunyu...@huawei.com> Created this test
> +*
> +*******************************************************************************************/
All above can be removed as it's stored in git history.
> +#define _GNU_SOURCE
> +#include <sys/wait.h>
> +#include <assert.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include "test.h"
> +#include "libclone.h"
> +#include "userns_helper.h"
> +
> +char *TCID = "user_namespace1";
> +int TST_TOTAL = 1;
> +
> +#define OVERFLOWUID 65534
I'd suggest getting the value from /proc if possible, and rely on
hardcoded value only if that fails, for example in setup() by:
SAFE_FILE_SCANF()
> +
> +/*
> + * child_fn1() - Inside a new user namespace
> + */
> +static int child_fn1(void *arg)
> +{
> + int exit_val;
> + int uid, gid;
> + uid = geteuid();
> + gid = getegid();
> +
> + tst_resm(TINFO, "USERNS test is running in a new user namespace.");
> + if (uid == OVERFLOWUID && gid == OVERFLOWUID) {
> + printf("Got expected cpid and ppid\n");
The printf message needs updating, there's no cpid and ppid here.
> + exit_val = 0;
> + } else {
> + printf("Got unexpected result of uid=%d gid=%d\n", uid, gid);
> + exit_val = 1;
> + }
> +
> + return exit_val;
> +}
> +
> +static void setup(void)
> +{
> + check_newuser();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + int status;
> + tst_parse_opts(argc, argv, NULL, NULL);
> + setup();
> +
> + TEST(do_clone_unshare_test(T_CLONE, CLONE_NEWUSER, child_fn1, NULL));
> +
> + if (TEST_RETURN == -1) {
> + tst_brkm(TFAIL | TTERRNO, NULL, "clone failed");
> + } else if ((wait(&status)) == -1) {
> + tst_brkm(TWARN | TERRNO, NULL, "wait failed");
> + }
> +
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_resm(TFAIL, "child exited abnormally");
> + else if (WIFSIGNALED(status)) {
> + tst_resm(TFAIL, "child was killed with signal = %d",
> + WTERMSIG(status));
> + }
A TPASS message would be nice if everything goes as expected.
Regards,
Jan
> +
> + tst_exit();
> +}
> +
> diff --git a/testcases/kernel/containers/userns/userns_helper.h
> b/testcases/kernel/containers/userns/userns_helper.h
> new file mode 100644
> index 0000000..36e75ad
> --- /dev/null
> +++ b/testcases/kernel/containers/userns/userns_helper.h
> @@ -0,0 +1,37 @@
> +/*
> +* Copyright (c) International Business Machines Corp., 2007
> +* This program is free software; you can redistribute it and/or modify
> +* it under the terms of the GNU General Public License as published by
> +* the Free Software Foundation; either version 2 of the License, or
> +* (at your option) any later version.
> +*
> +* This program is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> +* the GNU General Public License for more details.
> +*/
> +
> +#include "../libclone/libclone.h"
> +#include "test.h"
> +#include "safe_macros.h"
> +
> +static int dummy_child(void *v)
> +{
> + (void) v;
> + return 0;
> +}
> +
> +static int check_newuser(void)
> +{
> + int pid, status;
> +
> + if (tst_kvercmp(3, 8, 0) < 0)
> + tst_brkm(TCONF, NULL, "CLONE_NEWUSER not supported");
> +
> + pid = do_clone_unshare_test(T_CLONE, CLONE_NEWUSER, dummy_child, NULL);
> + if (pid == -1)
> + tst_brkm(TCONF | TERRNO, NULL, "CLONE_NEWUSER not supported");
> + SAFE_WAIT(NULL, &status);
> +
> + return 0;
> +}
>
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
