Re: [LTP] [PATCH 2/3 v3] containers: added mountns dir and mountns/mountns01.c

[chrubis@suse.cz]

Hi!
> +#define _GNU_SOURCE
> +#include <sys/wait.h>
> +#include <sys/mount.h>
> +#include <stdio.h>
> +#include <errno.h>
> +#include "test.h"
> +#include "usctest.h"
> +#include "libclone.h"
> +#include "safe_macros.h"
> +#include "safe_file_ops.h"
> +#include "mountns_helper.h"
> +
> +
> +#define DIRA "A"
> +#define DIRB "B"
> +char *TCID	= "mountns01";
> +int TST_TOTAL	= 1;
> +
> +
> +/* checks if following mountflags are defined */

This comment is close to "commenting the obvious" category. Everybody
can tell what the preprocessor #if below do. I would personaly omit this
comment.

> +#if defined(MS_SHARED) && defined(MS_PRIVATE) && defined(MS_REC)
> +
> +static void cleanup(void)
> +{
> +	umount(DIRA);
> +	umount(DIRA);
> +	umount(DIRB);
> +	tst_rmdir();
> +}
> +
> +static void setup(void)
> +{
> +	tst_require_root(NULL);
> +	check_newns();	/* from mountns_helper.h */

Here as well. It's not a good idea to comment where functions are
implemented, it only makes the source code longer and less clear.


The rest of the test looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce.
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list