All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [LTP] [PATCH] Add userns01 testcase to verify user namespace.
       [not found] <1432063864-24858-1-git-send-email-sunyuan3@huawei.com>
@ 2015-05-19 12:25 ` Jan Stancek
  0 siblings, 0 replies; only message in thread
From: Jan Stancek @ 2015-05-19 12:25 UTC (permalink / raw)
  To: ltp-list

On 05/19/2015 09:31 PM, Yuan Sun wrote:
> Signed-off-by: Yuan Sun <sunyuan3@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 <sunyuan3@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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2015-05-19 12:25 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1432063864-24858-1-git-send-email-sunyuan3@huawei.com>
2015-05-19 12:25 ` [LTP] [PATCH] Add userns01 testcase to verify user namespace Jan Stancek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.