lib/tst_tmpdir.c: fix bug when TESTDIR is not a absolute pathname

PATCH In review Xiaoguang Wang 2 participants 2 messages Jul 23, 2014 – Jul 28, 2014 1 message not in the archive Library code

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Xiaoguang Wang

author

2014-07-23 06:50 UTC #1 lore raw

PATCH 1/1 lib/tst_tmpdir.c: fix bug when TESTDIR is not a absolute pathname

In lib/tst_tmpdir.c, when TESTDIR is not a absolute patchname beginning with '/', The corresponding code logic would be below:

if (TESTDIR[0] != '/') {
	if (getcwd(current_dir, PATH_MAX) == NULL)
		strncpy(parent_dir, TESTDIR, PATH_MAX);
	else
		sprintf(parent_dir, "%s/%s", current_dir, TESTDIR);
} else {
	...
}

The statement "sprintf(parent_dir, "%s/%s", current_dir, TESTDIR);" is wrong, because we can change to any directory, which TESTDIR may no be child directory of current_dir, fix this.

Steps to reproduce this bug:
This will output the below errors:

fallocate01    1  TPASS  :  fallocate(3, 0, 49152, 4096) returned 0
fallocate01    2  TPASS  :  write operation on fallocated(3, 0, 49152, 4096) returned 1
fallocate01    3  TPASS  :  fallocate(4, 1, 49152, 4096) returned 0
fallocate01    4  TPASS  :  write operation on fallocated(4, 1, 49152, 4096) returned 1
fallocate01    0  TWARN  :  tst_rmdir: chdir(/home/lege/extern_stroage/ltp-dev/testcases/kernel/syscalls/fallocate/testdir/falkmsya7/testdir) failed

Attempting to remove temp dir anyway: errno=ENOENT(2): No such file or directory
fallocate01 0 TWARN : tst_rmdir: rmobj(testdir/falkmsya7) failed: lstat(testdir/falkmsya7) failed; errno=2: No such file or directory

Signed-off-by Xiaoguang Wang <[email protected]>

---
 lib/tst_tmpdir.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

lib/tst_tmpdir.c +22 ~~-20~~

index 600d178..cd4b39a 100644
@@ -148,8 +148,8 @@ void tst_tmpdir(void)
 			 "chmod(%s, %#o) failed", TESTDIR, DIR_MODE);
 
 	if (getcwd(test_start_work_dir, sizeof(test_start_work_dir)) == NULL) {
-		tst_resm(TINFO, "Failed to record test working dir");
-		test_start_work_dir[0] = '\0';
+		tst_brkm(TBROK | TERRNO, tmpdir_cleanup,
+			 "Failed to record test working dir");
 	}
 
 	/*

@@ -173,7 +173,6 @@ void tst_tmpdir(void)
 
 void tst_rmdir(void)
 {
-	char current_dir[PATH_MAX];
 	char *errmsg;
 	char *parent_dir;

@@ -198,26 +197,29 @@ void tst_rmdir(void)
 	 * get full path.
 	 */
 	if (TESTDIR[0] != '/') {
-		if (getcwd(current_dir, PATH_MAX) == NULL)
-			strncpy(parent_dir, TESTDIR, PATH_MAX);
-		else
-			sprintf(parent_dir, "%s/%s", current_dir, TESTDIR);
+		if (chdir(test_start_work_dir) != 0) {
+			tst_resm(TWARN | TERRNO, "%s: chdir(%s) failed\n"
+				 "Attempting to remove temp dir anyway",
+				 __func__, test_start_work_dir);
+		}
 	} else {
 		strcpy(parent_dir, TESTDIR);
-	}
 
-	if ((parent_dir = dirname(parent_dir)) == NULL) {
-		tst_resm(TWARN | TERRNO, "%s: dirname failed", __func__);
-		return;
-	}
-
-	/*
-	 * Change directory to parent_dir (The dir above TESTDIR).
-	 */
-	if (chdir(parent_dir) != 0) {
-		tst_resm(TWARN | TERRNO,
-			 "%s: chdir(%s) failed\nAttempting to remove temp dir "
-			 "anyway", __func__, parent_dir);
+		parent_dir = dirname(parent_dir);
+		if (parent_dir == NULL) {
+			tst_resm(TWARN | TERRNO,
+				 "%s: dirname failed", __func__);
+			return;
+		}
+
+		/*
+		 * Change directory to parent_dir (The dir above TESTDIR).
+		 */
+		if (chdir(parent_dir) != 0) {
+			tst_resm(TWARN | TERRNO, "%s: chdir(%s) failed\n"
+				 "Attempting to remove temp dir anyway",
+				 __func__, parent_dir);
+		}
 	}
 
 	/*

Signature · 13 lines

1.8.2.1


------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

[email protected]

2014-07-28 13:51 UTC #2 lore raw

Hi!

Quoted 62 lines

---
 lib/tst_tmpdir.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

lib/tst_tmpdir.c +13 ~~-20~~

index 600d178..cd4b39a 100644
@@ -148,8 +148,8 @@ void tst_tmpdir(void)
 			 "chmod(%s, %#o) failed", TESTDIR, DIR_MODE);
 
 	if (getcwd(test_start_work_dir, sizeof(test_start_work_dir)) == NULL) {
-		tst_resm(TINFO, "Failed to record test working dir");
-		test_start_work_dir[0] = '\0';
+		tst_brkm(TBROK | TERRNO, tmpdir_cleanup,
+			 "Failed to record test working dir");
 	}
 
 	/*

@@ -173,7 +173,6 @@ void tst_tmpdir(void)
 
 void tst_rmdir(void)
 {
-	char current_dir[PATH_MAX];
 	char *errmsg;
 	char *parent_dir;

@@ -198,26 +197,29 @@ void tst_rmdir(void)
 	 * get full path.
 	 */
 	if (TESTDIR[0] != '/') {
-		if (getcwd(current_dir, PATH_MAX) == NULL)
-			strncpy(parent_dir, TESTDIR, PATH_MAX);
-		else
-			sprintf(parent_dir, "%s/%s", current_dir, TESTDIR);
+		if (chdir(test_start_work_dir) != 0) {
+			tst_resm(TWARN | TERRNO, "%s: chdir(%s) failed\n"
+				 "Attempting to remove temp dir anyway",
+				 __func__, test_start_work_dir);
+		}
 	} else {
 		strcpy(parent_dir, TESTDIR);
-	}
 
-	if ((parent_dir = dirname(parent_dir)) == NULL) {
-		tst_resm(TWARN | TERRNO, "%s: dirname failed", __func__);
-		return;
-	}
-
-	/*
-	 * Change directory to parent_dir (The dir above TESTDIR).
-	 */
-	if (chdir(parent_dir) != 0) {
-		tst_resm(TWARN | TERRNO,
-			 "%s: chdir(%s) failed\nAttempting to remove temp dir "
-			 "anyway", __func__, parent_dir);
+		parent_dir = dirname(parent_dir);
+		if (parent_dir == NULL) {
+			tst_resm(TWARN | TERRNO,
+				 "%s: dirname failed", __func__);
+			return;
+		}

lib/tst_tmpdir.c void tst_rmdir(void) · line 223

Quoted 10 lines

lib/tst_tmpdir.c +9 -0

+
+		/*
+		 * Change directory to parent_dir (The dir above TESTDIR).
+		 */
+		if (chdir(parent_dir) != 0) {
+			tst_resm(TWARN | TERRNO, "%s: chdir(%s) failed\n"
+				 "Attempting to remove temp dir anyway",
+				 __func__, parent_dir);
+		}
 	}

Good catch.

Can't we just do chdir(test_start_work_dir) in both cases and then the rmobj(TESTDIR, &errmsg)? That would really simplify the code for tst_rmdir().

Signature · 13 lines

Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list