Re: [LTP] [PATCH 2/6] Add mkdtemp check.

Re: [LTP] [PATCH 2/6] Add mkdtemp check.

[Wanlong Gao]

On 07/13/2012 11:51 PM, Marios Makris wrote:
> From: Cyril Hrubis <metan@ucw.cz>
> 
> Added an m4 macro checking for mkdtemp()
> 
> Signed-off-by: Cyril Hrubis <metan@ucw.cz>
> Signed-off-by: Marios Makris <marios.makris@gmail.com>

Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>

> ---
>  configure.ac      |    1 +
>  m4/ltp-mkdtemp.m4 |    2 ++
>  2 files changed, 3 insertions(+)
>  create mode 100644 m4/ltp-mkdtemp.m4
> 
> diff --git a/configure.ac b/configure.ac
> index 1696df0..f217f50 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -145,6 +145,7 @@ fi
>  
>  # END testsuites knobs
>  
> +LTP_CHECK_MKDTEMP
>  LTP_CHECK_CAPABILITY_SUPPORT
>  LTP_CHECK_CRYPTO
>  LTP_CHECK_LINUX_PTRACE
> diff --git a/m4/ltp-mkdtemp.m4 b/m4/ltp-mkdtemp.m4
> new file mode 100644
> index 0000000..94a6d49
> --- /dev/null
> +++ b/m4/ltp-mkdtemp.m4
> @@ -0,0 +1,2 @@
> +AC_DEFUN([LTP_CHECK_MKDTEMP],[
> +AC_CHECK_FUNCS(mkdtemp,[],AC_MSG_ERROR(mkdtemp() not found!))])
> 



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 3/6] /lib/tst_tmpdir.c removed mkstemp method

[Wanlong Gao]

On 07/13/2012 11:51 PM, Marios Makris wrote:
> Removed the mkstemp method keeping only the mkdtemp since it was added in
> glibc in 2000, therefore it should be available in every linux distro.
> 
> Signed-off-by: Marios Makris <marios.makris@gmail.com>

Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>

> ---
>  lib/tst_tmpdir.c |   37 -------------------------------------
>  1 file changed, 37 deletions(-)
> 
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index fa7628d..b6f4d2e 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -112,10 +112,6 @@ void tst_tmpdir(void)
>  	char template[PATH_MAX];
>  	int  no_cleanup = 0;	/* !0 means TDIRECTORY env var was set */
>  	char *env_tmpdir;	/* temporary storage for TMPDIR env var */
> -	/* This is an AWFUL hack to figure out if mkdtemp() is available */
> -#if defined(__GLIBC_PREREQ) && __GLIBC_PREREQ(2, 2)
> -#define HAVE_MKDTEMP
> -#endif
>  
>  	/*
>  	 * If the TDIRECTORY env variable is not set, a temp dir will be
> @@ -150,7 +146,6 @@ void tst_tmpdir(void)
>  			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
>  				TEMPDIR, TCID);
>  
> -#ifdef HAVE_MKDTEMP
>  		/* Make the temporary directory in one shot using mkdtemp. */
>  		if (mkdtemp(template) == NULL)
>  			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> @@ -159,38 +154,6 @@ void tst_tmpdir(void)
>  			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
>  				"%s: strdup(%s) failed", __func__, template);
>  		}
> -#else
> -		int tfd;
> -
> -		/* Make the template name, then the directory */
> -		if ((tfd = mkstemp(template)) == -1)
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: mkstemp(%s) failed", __func__, template);
> -		if (close(tfd) == -1) {
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: close() failed", __func__);
> -		}
> -		if (unlink(template) == -1) {
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: unlink(%s) failed", __func__, template);
> -		}
> -		if ((TESTDIR = strdup(template)) == NULL) {
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: strdup(%s) failed", __func__, template);
> -		}
> -		if (mkdir(TESTDIR, DIR_MODE)) {
> -			/*
> -			 * If we start failing with EEXIST, wrap this section in
> -			 * a loop so we can try again.
> -			 *
> -			 * XXX (garrcoop): why? Hacking around broken
> -			 * filesystems should not be done.
> -			 */
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: mkdir(%s, %#o) failed",
> -					__func__, TESTDIR, DIR_MODE);
> -		}
> -#endif
>  
>  		if (chown(TESTDIR, -1, getgid()) == -1)
>  			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> 



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/6] /lib/tst_tmpdir indentation and coding style fixes

[Wanlong Gao]

On 07/13/2012 11:51 PM, Marios Makris wrote:
> Identation and coding style fixes as well as moved the function descriptions
> to the header file (/include/test.h)
> 
> Signed-off-by: Marios Makris <marios.makris@gmail.com>
> ---
>  include/test.h   |   26 +++++++++++++++
>  lib/tst_tmpdir.c |   97 ++++++++++++++++++------------------------------------
>  2 files changed, 58 insertions(+), 65 deletions(-)
> 
> diff --git a/include/test.h b/include/test.h
> index 033fdbc..934585e 100644
> --- a/include/test.h
> +++ b/include/test.h
> @@ -201,8 +201,34 @@ extern int Tst_count;
>  void tst_sig(int fork_flag, void (*handler)(), void (*cleanup)());
>  
>  /* lib/tst_tmpdir.c */
> +
> +/* tst_tmpdir()
> + *
> + * Create a unique temporary directory and chdir() to it. It expects the caller

It's better to keep the comments lines within 73 characters.

Thanks,
Wanlong Gao

> + * to have defined/initialized the TCID/TST_TOTAL global variables.
> + * The TESTDIR global variable will be set to the directory that gets used
> + * as the testing directory.
> + *
> + * NOTE: This function must be called BEFORE any activity that would require
> + * CLEANUP.  If tst_tmpdir() fails, it cleans up afer itself and calls
> + * tst_exit() (i.e. does not return).
> + */
>  void tst_tmpdir(void);
> +/* tst_rmdir()
> + *
> + * Recursively remove the temporary directory created by tst_tmpdir().
> + * This function is intended ONLY as a companion to tst_tmpdir().
> + * If the TDIRECTORY environment variable is set, no cleanup will be
> + * attempted.
> + */
>  void tst_rmdir(void);
> +/* get_tst_tmpdir()
> + *
> + * Return a copy of the test temp directory as seen by LTP. This is for
> + * path-oriented tests like chroot, etc, that may munge the path a bit.
> + *
> + * FREE VARIABLE AFTER USE IF IT IS REUSED!
> + */
>  char *get_tst_tmpdir(void);
>  
>  /* lib/get_high_address.c */
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index d112e3e..fa7628d 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -1,4 +1,4 @@
> -/*
> +/**********************************************************
>   * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -28,9 +28,7 @@
>   * For further information regarding this notice, see:
>   *
>   * http://oss.sgi.com/projects/GenInfo/NoticeExplan/
> - */
> -
> -/* $Id: tst_tmpdir.c,v 1.14 2009/07/20 10:59:32 vapier Exp $ */
> + *********************************************************/
>  
>  /**********************************************************
>   *
> @@ -62,7 +60,8 @@
>   *	 RETURN VALUE
>   *		Neither tst_tmpdir() or tst_rmdir() has a return value.
>   *
> - *#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#*#**/
> + *********************************************************/
> +
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <assert.h>
> @@ -72,6 +71,7 @@
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> +
>  #include "test.h"
>  #include "rmobj.h"
>  
> @@ -99,14 +99,7 @@ static void tmpdir_cleanup(void);
>  extern char *TCID;		/* defined/initialized in main() */
>  static char *TESTDIR = NULL;	/* the directory created */
>  
> -/*
> - * Return a copy of the test temp directory as seen by LTP. This is for
> - * path-oriented tests like chroot, etc, that may munge the path a bit.
> - *
> - * FREE VARIABLE AFTER USE IF IT IS REUSED!
> - */
> -char *
> -get_tst_tmpdir(void)
> +char *get_tst_tmpdir(void)
>  {
>  	/* Smack the user for calling things out of order. */
>  	if (TESTDIR == NULL)
> @@ -114,25 +107,13 @@ get_tst_tmpdir(void)
>  	return strdup(TESTDIR);
>  }
>  
> -/*
> - * tst_tmpdir() - Create a unique temporary directory and chdir() to it.
> - *		It expects the caller to have defined/initialized the
> - *		TCID/TST_TOTAL global variables.  The TESTDIR global
> - *		variable will be set to the directory that gets used
> - *		as the testing directory.
> - *
> - *		NOTE: This function must be called BEFORE any activity
> - *		that would require CLEANUP.  If tst_tmpdir() fails, it
> - *		cleans up afer itself and calls tst_exit() (i.e. does
> - *		not return).
> - */
>  void tst_tmpdir(void)
>  {
>  	char template[PATH_MAX];
>  	int  no_cleanup = 0;	/* !0 means TDIRECTORY env var was set */
>  	char *env_tmpdir;	/* temporary storage for TMPDIR env var */
>  	/* This is an AWFUL hack to figure out if mkdtemp() is available */
> -#if defined(__GLIBC_PREREQ) && __GLIBC_PREREQ(2,2)
> +#if defined(__GLIBC_PREREQ) && __GLIBC_PREREQ(2, 2)
>  #define HAVE_MKDTEMP
>  #endif
>  
> @@ -146,7 +127,7 @@ void tst_tmpdir(void)
>  		 */
>  		if (mkdir(TESTDIR, DIR_MODE) == -1 && errno != EEXIST) {
>  			tst_brkm(TBROK, NULL, "mkdir(%s, %o) failed",
> -			    TESTDIR, DIR_MODE);
> +				TESTDIR, DIR_MODE);
>  		}
>  		/*
>  		 * Try to create the directory if it does not exist already;
> @@ -162,11 +143,12 @@ void tst_tmpdir(void)
>  		 * environment variable TMPDIR if it is available, otherwise
>  		 * use our default TEMPDIR.
>  		 */
> -		if ((env_tmpdir = getenv("TMPDIR"))) {
> -			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", env_tmpdir, TCID);
> -		} else {
> -			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX", TEMPDIR, TCID);
> -		}
> +		if ((env_tmpdir = getenv("TMPDIR")))
> +			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
> +				env_tmpdir, TCID);
> +		else
> +			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
> +				TEMPDIR, TCID);
>  
>  #ifdef HAVE_MKDTEMP
>  		/* Make the temporary directory in one shot using mkdtemp. */
> @@ -205,37 +187,37 @@ void tst_tmpdir(void)
>  			 * filesystems should not be done.
>  			 */
>  			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -			    "%s: mkdir(%s, %#o) failed",
> -			    __func__, TESTDIR, DIR_MODE);
> +				"%s: mkdir(%s, %#o) failed",
> +					__func__, TESTDIR, DIR_MODE);
>  		}
>  #endif
>  
>  		if (chown(TESTDIR, -1, getgid()) == -1)
>  			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -			    "chown(%s, -1, %d) failed", TESTDIR, getgid());
> +				"chown(%s, -1, %d) failed", TESTDIR, getgid());
>  		if (chmod(TESTDIR, DIR_MODE) == -1)
>  			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
>  				"chmod(%s, %#o) failed", TESTDIR, DIR_MODE);
> - 	}
> +	}
>  
>  #if UNIT_TEST
>  	printf("TESTDIR = %s\n", TESTDIR);
>  #endif
>  
> - 	/*
> -  	 * Change to the temporary directory.  If the chdir() fails, issue
> +	/*
> +	 * Change to the temporary directory.  If the chdir() fails, issue
>  	 * TBROK messages for all test cases, attempt to remove the
>  	 * directory (if it was created), and exit.  If the removal also
>  	 * fails, also issue a TWARN message.
>  	 */
>  	if (chdir(TESTDIR) == -1) {
>  		tst_brkm(TBROK|TERRNO, NULL, "%s: chdir(%s) failed",
> -		    __func__, TESTDIR);
> +			__func__, TESTDIR);
>  
>  		/* Try to remove the directory */
>  		if (!no_cleanup && rmdir(TESTDIR) == -1) {
>  			tst_resm(TWARN|TERRNO, "%s: rmdir(%s) failed",
> -			    __func__, TESTDIR);
> +				__func__, TESTDIR);
>  		}
>  
>  		tmpdir_cleanup();
> @@ -245,17 +227,8 @@ void tst_tmpdir(void)
>  	printf("CWD is %s\n", getcwd(NULL, PATH_MAX));
>  #endif
>  
> -}  /* tst_tmpdir() */
> -
> +}
>  
> -/*
> - *
> - * tst_rmdir() - Recursively remove the temporary directory created by
> - *			 tst_tmpdir().  This function is intended ONLY as a
> - *			 companion to tst_tmpdir().  If the TDIRECTORY
> - *			 environment variable is set, no cleanup will be
> - *			 attempted.
> - */
>  void tst_rmdir(void)
>  {
>  	struct stat buf1;
> @@ -282,31 +255,26 @@ void tst_rmdir(void)
>  	 */
>  	if (TESTDIR == NULL) {
>  		tst_resm(TWARN,
> -		    "%s: TESTDIR was NULL; no removal attempted", __func__);
> +			"%s: TESTDIR was NULL; no removal attempted", __func__);
>  		return;
>  	}
>  
>  	if ((parent_dir = malloc(PATH_MAX)) == NULL) {
>  		/* Make sure that we exit quickly and noisily. */
>  		tst_brkm(TBROK|TERRNO, NULL,
> -		    "%s: malloc(%d) failed", __func__, PATH_MAX);
> +			"%s: malloc(%d) failed", __func__, PATH_MAX);
>  	}
>  
>  	/*
>  	 * Check that the value of TESTDIR is not "*" or "/".  These could
>  	 * have disastrous effects in a test run by root.
>  	 */
> -	/* XXX: a temp directory that's '/' seems stupid and dangerous anyways. */
>  	if (stat(TESTDIR, &buf1) == 0 && stat("/", &buf2) == 0 &&
> -	    buf1.st_ino == buf2.st_ino) {
> +		buf1.st_ino == buf2.st_ino) {
>  		tst_resm(TWARN, "%s: will not remove /", __func__);
>  		return;
>  	}
>  
> -	/*
> -	 * XXX: this just reeks of bad programming; all shell characters should
> -	 * be escaped in invocations of rm(1)/rmdir(1).
> -	 */
>  	if (strchr(TESTDIR, '*') != NULL) {
>  		tst_resm(TWARN, "%s: will not remove *", __func__);
>  		return;
> @@ -324,6 +292,7 @@ void tst_rmdir(void)
>  	} else {
>  		strcpy(parent_dir, TESTDIR);
>  	}
> +
>  	if ((parent_dir = dirname(parent_dir)) == NULL) {
>  		tst_resm(TWARN|TERRNO, "%s: dirname failed", __func__);
>  		return;
> @@ -334,9 +303,8 @@ void tst_rmdir(void)
>  	 */
>  	if (chdir(parent_dir) != 0) {
>  		tst_resm(TWARN|TERRNO,
> -			 "%s: chdir(%s) failed\nAttempting to remove temp dir "
> -			 "anyway",
> -			 __func__, parent_dir);
> +			"%s: chdir(%s) failed\nAttempting to remove temp dir "
> +				"anyway", __func__, parent_dir);
>  	}
>  
>  	/*
> @@ -344,9 +312,9 @@ void tst_rmdir(void)
>  	 */
>  	if (rmobj(TESTDIR, &errmsg) == -1)
>  		tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
> -			 __func__, TESTDIR, errmsg);
> +			__func__, TESTDIR, errmsg);
>  
> -}  /* tst_rmdir() */
> +}
>  
>  
>  /*
> @@ -355,8 +323,7 @@ void tst_rmdir(void)
>   *			  It prints a warning message via tst_resm(), and
>   *			  then calls tst_exit().
>   */
> -static void
> -tmpdir_cleanup(void)
> +static void tmpdir_cleanup(void)
>  {
>  	tst_brkm(TWARN, NULL,
>  	    "%s: no user cleanup function called before exiting", __func__);
> 



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 4/6] /lib/tst_tmpdir.c removed / and * checks

[Wanlong Gao]

On 07/13/2012 11:51 PM, Marios Makris wrote:
> These checks are implemented in the rmobj.c file therefore there is no
> need to repeat them here since rmobj() is called for deletions.
> 
> Signed-off-by: Marios Makris <marios.makris@gmail.com>
> ---
>  lib/tst_tmpdir.c |   17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index b6f4d2e..a9dd78c 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -194,8 +194,6 @@ void tst_tmpdir(void)
>  
>  void tst_rmdir(void)
>  {
> -	struct stat buf1;
> -	struct stat buf2;
>  	char current_dir[PATH_MAX];
>  	char *errmsg;
>  	char *parent_dir;
> @@ -229,21 +227,6 @@ void tst_rmdir(void)
>  	}
>  
>  	/*
> -	 * Check that the value of TESTDIR is not "*" or "/".  These could
> -	 * have disastrous effects in a test run by root.
> -	 */
> -	if (stat(TESTDIR, &buf1) == 0 && stat("/", &buf2) == 0 &&
> -		buf1.st_ino == buf2.st_ino) {
> -		tst_resm(TWARN, "%s: will not remove /", __func__);
> -		return;
> -	}
> -
> -	if (strchr(TESTDIR, '*') != NULL) {
> -		tst_resm(TWARN, "%s: will not remove *", __func__);
> -		return;
> -	}

Seems that rmobj() doesn't check the "*" there?

Thanks,
Wanlong Gao

> -
> -	/*
>  	 * Get the directory name of TESTDIR.  If TESTDIR is a relative path,
>  	 * get full path.
>  	 */
> 



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 5/6] /lib/tst_tmpdir.c removed the TDIRECTORY part

[Wanlong Gao]

On 07/13/2012 11:51 PM, Marios Makris wrote:
> After recent question to the list, it seems that the TDIRECTORY PART is not
> used by anyone, therefore it is removed.

OK, we can also use "-d TMPDIR" option, so no objection with removing this.
 
> 
> Signed-off-by: Marios Makris <marios.makris@gmail.com>

Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>

Thanks,
Wanlong Gao

> ---
>  lib/tst_tmpdir.c |   93 ++++++++++++++++--------------------------------------
>  1 file changed, 28 insertions(+), 65 deletions(-)
> 
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index a9dd78c..080e09a 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -110,58 +110,35 @@ char *get_tst_tmpdir(void)
>  void tst_tmpdir(void)
>  {
>  	char template[PATH_MAX];
> -	int  no_cleanup = 0;	/* !0 means TDIRECTORY env var was set */
>  	char *env_tmpdir;	/* temporary storage for TMPDIR env var */
> +	char *errmsg;
>  
>  	/*
> -	 * If the TDIRECTORY env variable is not set, a temp dir will be
> -	 * created.
> +	 * Create a template for the temporary directory.  Use the
> +	 * environment variable TMPDIR if it is available, otherwise
> +	 * use our default TEMPDIR.
>  	 */
> -	if ((TESTDIR = getenv(TDIRECTORY)) != NULL) {
> -		/*
> -		 * The TDIRECTORY env. variable is set, so no temp dir is created.
> -		 */
> -		if (mkdir(TESTDIR, DIR_MODE) == -1 && errno != EEXIST) {
> -			tst_brkm(TBROK, NULL, "mkdir(%s, %o) failed",
> -				TESTDIR, DIR_MODE);
> -		}
> -		/*
> -		 * Try to create the directory if it does not exist already;
> -		 * user might forget to create one before exporting TDIRECTORY.
> -		 */
> -		no_cleanup++;
> -#if UNIT_TEST
> -		printf("TDIRECTORY env var is set\n");
> -#endif
> -	} else {
> -		/*
> -		 * Create a template for the temporary directory.  Use the
> -		 * environment variable TMPDIR if it is available, otherwise
> -		 * use our default TEMPDIR.
> -		 */
> -		if ((env_tmpdir = getenv("TMPDIR")))
> -			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
> -				env_tmpdir, TCID);
> -		else
> -			snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
> -				TEMPDIR, TCID);
> -
> -		/* Make the temporary directory in one shot using mkdtemp. */
> -		if (mkdtemp(template) == NULL)
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: mkdtemp(%s) failed", __func__, template);
> -		if ((TESTDIR = strdup(template)) == NULL) {
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"%s: strdup(%s) failed", __func__, template);
> -		}
> -
> -		if (chown(TESTDIR, -1, getgid()) == -1)
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"chown(%s, -1, %d) failed", TESTDIR, getgid());
> -		if (chmod(TESTDIR, DIR_MODE) == -1)
> -			tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> -				"chmod(%s, %#o) failed", TESTDIR, DIR_MODE);
> -	}
> +	if ((env_tmpdir = getenv("TMPDIR")))
> +		snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
> +			env_tmpdir, TCID);
> +	else
> +		snprintf(template, PATH_MAX, "%s/%.3sXXXXXX",
> +			TEMPDIR, TCID);
> +
> +	/* Make the temporary directory in one shot using mkdtemp. */
> +	if (mkdtemp(template) == NULL)
> +		tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> +			"%s: mkdtemp(%s) failed", __func__, template);
> +	if ((TESTDIR = strdup(template)) == NULL)
> +		tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> +			"%s: strdup(%s) failed", __func__, template);
> +
> +	if (chown(TESTDIR, -1, getgid()) == -1)
> +		tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> +			"chown(%s, -1, %d) failed", TESTDIR, getgid());
> +	if (chmod(TESTDIR, DIR_MODE) == -1)
> +		tst_brkm(TBROK|TERRNO, tmpdir_cleanup,
> +			"chmod(%s, %#o) failed", TESTDIR, DIR_MODE);
>  
>  #if UNIT_TEST
>  	printf("TESTDIR = %s\n", TESTDIR);
> @@ -178,10 +155,9 @@ void tst_tmpdir(void)
>  			__func__, TESTDIR);
>  
>  		/* Try to remove the directory */
> -		if (!no_cleanup && rmdir(TESTDIR) == -1) {
> -			tst_resm(TWARN|TERRNO, "%s: rmdir(%s) failed",
> -				__func__, TESTDIR);
> -		}
> +		if (rmobj(TESTDIR, &errmsg) == -1)
> +			tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
> +			__func__, TESTDIR, errmsg);
>  
>  		tmpdir_cleanup();
>  	}
> @@ -197,19 +173,6 @@ void tst_rmdir(void)
>  	char current_dir[PATH_MAX];
>  	char *errmsg;
>  	char *parent_dir;
> -	char *tdirectory;
> -
> -	/*
> -	 * If the TDIRECTORY env variable is set, this indicates that no
> -	 * temp dir was created by tst_tmpdir().  Thus no cleanup will be
> -	 * necessary.
> -	 */
> -	if ((tdirectory = getenv(TDIRECTORY)) != NULL) {
> -#if UNIT_TEST
> -		printf("\"TDIRECORY\" env variable is set; no cleanup was performed\n");
> -#endif
> -		return;
> -	}
>  
>  	/*
>  	 * Check that TESTDIR is not NULL.
> 



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 4/6] /lib/tst_tmpdir.c removed / and * checks

[marios.makris]

On 18 July 2012 12:32, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> On 07/13/2012 11:51 PM, Marios Makris wrote:
>> These checks are implemented in the rmobj.c file therefore there is no
>> need to repeat them here since rmobj() is called for deletions.
>>
>> Signed-off-by: Marios Makris <marios.makris@gmail.com>
>> ---
>>  lib/tst_tmpdir.c |   17 -----------------
>>  1 file changed, 17 deletions(-)
>>
>> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
>> index b6f4d2e..a9dd78c 100644
>> --- a/lib/tst_tmpdir.c
>> +++ b/lib/tst_tmpdir.c
>> @@ -194,8 +194,6 @@ void tst_tmpdir(void)
>>
>>  void tst_rmdir(void)
>>  {
>> -     struct stat buf1;
>> -     struct stat buf2;
>>       char current_dir[PATH_MAX];
>>       char *errmsg;
>>       char *parent_dir;
>> @@ -229,21 +227,6 @@ void tst_rmdir(void)
>>       }
>>
>>       /*
>> -      * Check that the value of TESTDIR is not "*" or "/".  These could
>> -      * have disastrous effects in a test run by root.
>> -      */
>> -     if (stat(TESTDIR, &buf1) == 0 && stat("/", &buf2) == 0 &&
>> -             buf1.st_ino == buf2.st_ino) {
>> -             tst_resm(TWARN, "%s: will not remove /", __func__);
>> -             return;
>> -     }
>> -
>> -     if (strchr(TESTDIR, '*') != NULL) {
>> -             tst_resm(TWARN, "%s: will not remove *", __func__);
>> -             return;
>> -     }
>
> Seems that rmobj() doesn't check the "*" there?
>
> Thanks,
> Wanlong Gao
>

Hi and thanks for reviewing.

Well rmobj doesn't check for "*", but it uses remove and unlink and
if i am not wrong they don't do the "*" expansion?



-- 
Marios Makris

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/6] /lib/tst_tmpdir indentation and coding style fixes

[marios.makris]

On 18 July 2012 11:20, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> On 07/13/2012 11:51 PM, Marios Makris wrote:
>> Identation and coding style fixes as well as moved the function descriptions
>> to the header file (/include/test.h)
>>
>> Signed-off-by: Marios Makris <marios.makris@gmail.com>
>> ---
>>  include/test.h   |   26 +++++++++++++++
>>  lib/tst_tmpdir.c |   97 ++++++++++++++++++------------------------------------
>>  2 files changed, 58 insertions(+), 65 deletions(-)
>>
>> diff --git a/include/test.h b/include/test.h
>> index 033fdbc..934585e 100644
>> --- a/include/test.h
>> +++ b/include/test.h
>> @@ -201,8 +201,34 @@ extern int Tst_count;
>>  void tst_sig(int fork_flag, void (*handler)(), void (*cleanup)());
>>
>>  /* lib/tst_tmpdir.c */
>> +
>> +/* tst_tmpdir()
>> + *
>> + * Create a unique temporary directory and chdir() to it. It expects the caller
>
> It's better to keep the comments lines within 73 characters.
>
> Thanks,
> Wanlong Gao
>

Well i thought that the line limit was 80 characters(columns) ?

Thanks

-- 
Marios Makris

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/6] /lib/tst_tmpdir indentation and coding style fixes

[Caspar Zhang]

On 07/18/2012 09:32 PM, marios.makris wrote:
> On 18 July 2012 11:20, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
>> On 07/13/2012 11:51 PM, Marios Makris wrote:
>>> Identation and coding style fixes as well as moved the function descriptions
>>> to the header file (/include/test.h)
>>>
>>> Signed-off-by: Marios Makris <marios.makris@gmail.com>
>>> ---
>>>   include/test.h   |   26 +++++++++++++++
>>>   lib/tst_tmpdir.c |   97 ++++++++++++++++++------------------------------------
>>>   2 files changed, 58 insertions(+), 65 deletions(-)
>>>
>>> diff --git a/include/test.h b/include/test.h
>>> index 033fdbc..934585e 100644
>>> --- a/include/test.h
>>> +++ b/include/test.h
>>> @@ -201,8 +201,34 @@ extern int Tst_count;
>>>   void tst_sig(int fork_flag, void (*handler)(), void (*cleanup)());
>>>
>>>   /* lib/tst_tmpdir.c */
>>> +
>>> +/* tst_tmpdir()
>>> + *
>>> + * Create a unique temporary directory and chdir() to it. It expects the caller
>>
>> It's better to keep the comments lines within 73 characters.
>>
>> Thanks,
>> Wanlong Gao
>>
>
> Well i thought that the line limit was 80 characters(columns) ?
>
> Thanks
>

I think it's 80 characters. (it should be OK if checkpatch.pl doesn't 
complain...)

Caspar


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/6] /lib/tst_tmpdir indentation and coding style fixes

[marios.makris]

On 18 July 2012 16:51, Caspar Zhang <caspar@casparzhang.com> wrote:
> On 07/18/2012 09:32 PM, marios.makris wrote:
>>
>> On 18 July 2012 11:20, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
>>>
>>> On 07/13/2012 11:51 PM, Marios Makris wrote:
>>>>
>>>> Identation and coding style fixes as well as moved the function
>>>> descriptions
>>>> to the header file (/include/test.h)
>>>>
>>>> Signed-off-by: Marios Makris <marios.makris@gmail.com>
>>>> ---
>>>>   include/test.h   |   26 +++++++++++++++
>>>>   lib/tst_tmpdir.c |   97
>>>> ++++++++++++++++++------------------------------------
>>>>   2 files changed, 58 insertions(+), 65 deletions(-)
>>>>
>>>> diff --git a/include/test.h b/include/test.h
>>>> index 033fdbc..934585e 100644
>>>> --- a/include/test.h
>>>> +++ b/include/test.h
>>>> @@ -201,8 +201,34 @@ extern int Tst_count;
>>>>   void tst_sig(int fork_flag, void (*handler)(), void (*cleanup)());
>>>>
>>>>   /* lib/tst_tmpdir.c */
>>>> +
>>>> +/* tst_tmpdir()
>>>> + *
>>>> + * Create a unique temporary directory and chdir() to it. It expects
>>>> the caller
>>>
>>>
>>> It's better to keep the comments lines within 73 characters.
>>>
>>> Thanks,
>>> Wanlong Gao
>>>
>>
>> Well i thought that the line limit was 80 characters(columns) ?
>>
>> Thanks
>>
>
> I think it's 80 characters. (it should be OK if checkpatch.pl doesn't
> complain...)
>
> Caspar
>

Well there are 2 lines slightly over 80 characters but they are completely
removed in 5th patch with the TDIRECTORY part...

I could fix them in the first patch as-well if you think it would be better
but the end result (if all patches are applied) is the same.

Thanks


-- 
Marios Makris

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/6] /lib/tst_tmpdir indentation and coding style fixes

[Caspar Zhang]

On 07/18/2012 10:42 PM, marios.makris wrote:
> On 18 July 2012 16:51, Caspar Zhang <caspar@casparzhang.com> wrote:
>> On 07/18/2012 09:32 PM, marios.makris wrote:
>>>
>>> On 18 July 2012 11:20, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
>>>>
>>>> On 07/13/2012 11:51 PM, Marios Makris wrote:
>>>>>
>>>>> Identation and coding style fixes as well as moved the function
>>>>> descriptions
>>>>> to the header file (/include/test.h)
>>>>>
>>>>> Signed-off-by: Marios Makris <marios.makris@gmail.com>
>>>>> ---
>>>>>    include/test.h   |   26 +++++++++++++++
>>>>>    lib/tst_tmpdir.c |   97
>>>>> ++++++++++++++++++------------------------------------
>>>>>    2 files changed, 58 insertions(+), 65 deletions(-)
>>>>>
>>>>> diff --git a/include/test.h b/include/test.h
>>>>> index 033fdbc..934585e 100644
>>>>> --- a/include/test.h
>>>>> +++ b/include/test.h
>>>>> @@ -201,8 +201,34 @@ extern int Tst_count;
>>>>>    void tst_sig(int fork_flag, void (*handler)(), void (*cleanup)());
>>>>>
>>>>>    /* lib/tst_tmpdir.c */
>>>>> +
>>>>> +/* tst_tmpdir()
>>>>> + *
>>>>> + * Create a unique temporary directory and chdir() to it. It expects
>>>>> the caller
>>>>
>>>>
>>>> It's better to keep the comments lines within 73 characters.
>>>>
>>>> Thanks,
>>>> Wanlong Gao
>>>>
>>>
>>> Well i thought that the line limit was 80 characters(columns) ?
>>>
>>> Thanks
>>>
>>
>> I think it's 80 characters. (it should be OK if checkpatch.pl doesn't
>> complain...)
>>
>> Caspar
>>
>
> Well there are 2 lines slightly over 80 characters but they are completely
> removed in 5th patch with the TDIRECTORY part...
>
> I could fix them in the first patch as-well if you think it would be better
> but the end result (if all patches are applied) is the same.

either is ok for me. your choice.

Caspar

>
> Thanks
>
>



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list