kernel/syscalls/fallocate: fix array parameter degradation

Compare size_t size elements with file data and check if it's excpeted.
This patch depends on bff5968.

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

index a6d1159..2da0bcd 100644

Hi,

Quoted 14 lines · Wei,Jiangang

Compare size_t size elements with file data and check if it's excpeted.
This patch depends on bff5968.

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

index a6d1159..2da0bcd 100644

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 177

Quoted 5 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

@@ -174,7 +174,7 @@ static void test02(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size, 0, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, sizeof(exp_buf)/sizeof(exp_buf[0]));

Thank you for fixing it.

We could you use size variables already defined for 'exp_buf[buf_size]'.

testcases/kernel/syscalls/fallocate/fallocate04.c static void test03(void) · line 218

Quoted 10 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -214,7 +214,7 @@ static void test03(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size - 1, 0, block_size + 2);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, sizeof(exp_buf)/sizeof(exp_buf[0]));
 

Also buf_size variable here.

testcases/kernel/syscalls/fallocate/fallocate04.c static void test04(void) · line 254

Quoted 9 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -250,7 +250,7 @@ static void test04(void)
  	memcpy(exp_buf, tmp_buf, block_size);
  	memcpy(exp_buf + block_size, tmp_buf + size, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, sizeof(exp_buf)/sizeof(exp_buf[0]));
 

'size' variable here.

testcases/kernel/syscalls/fallocate/fallocate04.c static void test04(void) · line 256

Quoted 2 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

  	tst_resm(TPASS, "test-case succeeded");
  }

Now test04 fails because I forgot to set adjacent two bytes in 'exp_buf' to zero in test04(), it could be fixed as follows:

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -2

@@ -250,8 +249,8 @@ static void test04(void)
 
         memcpy(exp_buf, tmp_buf, block_size);
         memcpy(exp_buf + block_size, tmp_buf + size, block_size);
-
-       check_file_data(exp_buf);
+       exp_buf[block_size - 1] = exp_buf[block_size] = '\0';
+       check_file_data(exp_buf, size);
 
         tst_resm(TPASS, "test-case succeeded");
  }

I think it would be better to merge fallocate04 fixes in a single patch.

Thanks,
Alexey

Using 'sizeof' on array given as function argument returns size of a pointer, instead of the array's. So,
It needs to pass an additional parameter size_t size indicating the number of elements in the array.

Besides above,
Set adjacent two bytes in 'exp_buf' to zero in test04().

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -4

index 723c886..911bbe8 100644
@@ -98,9 +98,8 @@ static void setup(void)
  	get_blocksize();
  }
 
-static void check_file_data(const char exp_buf[])
+static void check_file_data(const char exp_buf[], size_t size)
  {
-	size_t size = sizeof(exp_buf);
  	char rbuf[size];
 
  	tst_resm(TINFO, "reading the file, compare with expected buffer");

@@ -175,7 +174,7 @@ static void test02(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size, 0, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -215,7 +214,7 @@ static void test03(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size - 1, 0, block_size + 2);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

Hi,

Quoted 51 lines · Wei,Jiangang

Using 'sizeof' on array given as function argument returns size of a pointer, instead of the array's. So,
It needs to pass an additional parameter size_t size indicating the number of elements in the array.

Besides above,
Set adjacent two bytes in 'exp_buf' to zero in test04().

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -4

index 723c886..911bbe8 100644
@@ -98,9 +98,8 @@ static void setup(void)
  	get_blocksize();
  }
 
-static void check_file_data(const char exp_buf[])
+static void check_file_data(const char exp_buf[], size_t size)
  {
-	size_t size = sizeof(exp_buf);
  	char rbuf[size];
 
  	tst_resm(TINFO, "reading the file, compare with expected buffer");

@@ -175,7 +174,7 @@ static void test02(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size, 0, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -215,7 +214,7 @@ static void test03(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size - 1, 0, block_size + 2);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -251,7 +250,8 @@ static void test04(void)
  	memcpy(exp_buf, tmp_buf, block_size);
  	memcpy(exp_buf + block_size, tmp_buf + size, block_size);
 

testcases/kernel/syscalls/fallocate/fallocate04.c static void test04(void) · line 257

Quoted 6 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -1

-	check_file_data(exp_buf);
+	exp_buf[block_size - 1] = exp_buf[block_size] = '\0';
+	check_file_data(exp_buf, size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

Patch applied, thank you!

Best regards,
Alexey

Quoted 53 lines · Alexey Kodanev

Hi,

Quoted 51 lines · Wei,Jiangang

Using 'sizeof' on array given as function argument returns size of a pointer, instead of the array's. So,
It needs to pass an additional parameter size_t size indicating the number of elements in the array.

Besides above,
Set adjacent two bytes in 'exp_buf' to zero in test04().

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -4

index 723c886..911bbe8 100644
@@ -98,9 +98,8 @@ static void setup(void)
  	get_blocksize();
  }
 
-static void check_file_data(const char exp_buf[])
+static void check_file_data(const char exp_buf[], size_t size)
  {
-	size_t size = sizeof(exp_buf);
  	char rbuf[size];
 
  	tst_resm(TINFO, "reading the file, compare with expected buffer");

@@ -175,7 +174,7 @@ static void test02(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size, 0, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -215,7 +214,7 @@ static void test03(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size - 1, 0, block_size + 2);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -251,7 +250,8 @@ static void test04(void)
  	memcpy(exp_buf, tmp_buf, block_size);
  	memcpy(exp_buf + block_size, tmp_buf + size, block_size);
 

Quoted 8 lines

testcases/kernel/syscalls/fallocate/fallocate04.c static void test04(void) · line 257

Quoted 6 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -1

-	check_file_data(exp_buf);
+	exp_buf[block_size - 1] = exp_buf[block_size] = '\0';
+	check_file_data(exp_buf, size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

Patch applied, thank you!

Thank you for your feedback so quickly.
Do you have noticed the other commit?
->[PATCH 3/3] kernel/mem/mmapstress: fix resource leak

I guess it might have been ignored...

Thanks again,
Wei

Quoted 5 lines

Best regards,
Alexey

------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
.

Quoted 63 lines · Jiangang

Quoted 61 lines · Alexey Kodanev

Hi,

Quoted 48 lines · Wei,Jiangang

Using 'sizeof' on array given as function argument returns size of a pointer, instead of the array's. So,
It needs to pass an additional parameter size_t size indicating the number of elements in the array.

Besides above,
Set adjacent two bytes in 'exp_buf' to zero in test04().

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -4

index 723c886..911bbe8 100644
@@ -98,9 +98,8 @@ static void setup(void)
  	get_blocksize();
  }
 
-static void check_file_data(const char exp_buf[])
+static void check_file_data(const char exp_buf[], size_t size)
  {
-	size_t size = sizeof(exp_buf);
  	char rbuf[size];
 
  	tst_resm(TINFO, "reading the file, compare with expected buffer");

@@ -175,7 +174,7 @@ static void test02(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size, 0, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -215,7 +214,7 @@ static void test03(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size - 1, 0, block_size + 2);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

testcases/kernel/syscalls/fallocate/fallocate04.c static void test04(void) · line 257

Quoted 9 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -1

@@ -251,7 +250,8 @@ static void test04(void)
  	memcpy(exp_buf, tmp_buf, block_size);
  	memcpy(exp_buf + block_size, tmp_buf + size, block_size);
 
-	check_file_data(exp_buf);
+	exp_buf[block_size - 1] = exp_buf[block_size] = '\0';
+	check_file_data(exp_buf, size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

Patch applied, thank you!

Quoted 5 lines

Thank you for your feedback so quickly.
Do you have noticed the other commit?
->[PATCH 3/3] kernel/mem/mmapstress: fix resource leak

I guess it might have been ignored...

It's a bit messy, would you repost your patches which were not applied to make things more clearly?

Thanks,
Wanlong Gao

Quoted 9 lines

Thanks again,
Wei

Quoted 5 lines

Best regards,
Alexey

Quoted 12 lines

------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
.

.

Quoted 68 lines · Wanlong Gao

Quoted 67 lines · Jiangang

Quoted 53 lines · Alexey Kodanev

Hi,

Quoted 51 lines · Wei,Jiangang

Using 'sizeof' on array given as function argument returns size of a pointer, instead of the array's. So,
It needs to pass an additional parameter size_t size indicating the number of elements in the array.

Besides above,
Set adjacent two bytes in 'exp_buf' to zero in test04().

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -4

index 723c886..911bbe8 100644
@@ -98,9 +98,8 @@ static void setup(void)
  	get_blocksize();
  }
 
-static void check_file_data(const char exp_buf[])
+static void check_file_data(const char exp_buf[], size_t size)
  {
-	size_t size = sizeof(exp_buf);
  	char rbuf[size];
 
  	tst_resm(TINFO, "reading the file, compare with expected buffer");

@@ -175,7 +174,7 @@ static void test02(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size, 0, block_size);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -215,7 +214,7 @@ static void test03(void)
  	fill_tst_buf(exp_buf);
  	memset(exp_buf + block_size - 1, 0, block_size + 2);
 
-	check_file_data(exp_buf);
+	check_file_data(exp_buf, buf_size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

@@ -251,7 +250,8 @@ static void test04(void)
  	memcpy(exp_buf, tmp_buf, block_size);
  	memcpy(exp_buf + block_size, tmp_buf + size, block_size);
 

Quoted 8 lines

testcases/kernel/syscalls/fallocate/fallocate04.c static void test04(void) · line 257

Quoted 6 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -1

-	check_file_data(exp_buf);
+	exp_buf[block_size - 1] = exp_buf[block_size] = '\0';
+	check_file_data(exp_buf, size);
 
  	tst_resm(TPASS, "test-case succeeded");
  }

Patch applied, thank you!

Thank you for your feedback so quickly.
Do you have noticed the other commit?
->[PATCH 3/3] kernel/mem/mmapstress: fix resource leak

Quoted 4 lines

Quoted 1 line

I guess it might have been ignored...

It's a bit messy, would you repost your patches which were not applied to make things more clearly?

And start a new mail thread.

Thanks,
Wanlong Gao

Quoted 28 lines

Thanks,
Wanlong Gao

Quoted 9 lines

Thanks again,
Wei

Quoted 5 lines

Best regards,
Alexey

Quoted 12 lines

------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
.

.

Including memory and fd leak.

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/mem/mmapstress/mmapstress01.c | 7 +++++++
  1 file changed, 7 insertions(+)

testcases/kernel/mem/mmapstress/mmapstress01.c +0 -0

index 0baf0e2..1db8147 100644

testcases/kernel/mem/mmapstress/mmapstress01.c +2 -0

  			/*****	**	*****/
  			return 0;

@@ -668,6 +670,8 @@ int fileokay(char *file, uchar_t * expbuf)
  				(void)fprintf(stderr, "read %d of %ld bytes\n",
  					      (i * pagesize) + cnt,
  					      (long)mapsize);
+				free(readbuf);
+				close(fd);
  				return 0;
  			}
  		}

Quoted 7 lines · Wei,Jiangang

Including memory and fd leak.

Signed-off-by Wei,Jiangang <[email protected]>

---
  testcases/kernel/mem/mmapstress/mmapstress01.c | 7 +++++++
  1 file changed, 7 insertions(+)

testcases/kernel/mem/mmapstress/mmapstress01.c int fileokay(char *file, uchar_t * expbuf) · line 660

Quoted 9 lines

testcases/kernel/mem/mmapstress/mmapstress01.c +1 -0

index 0baf0e2..1db8147 100644
@@ -657,6 +657,8 @@ int fileokay(char *file, uchar_t * expbuf)
  			perror("read error");
  			/***** LTP Port *****/
  			local_flag = FAILED;
+			free(readbuf);

We could allocate readbuf on the stack... readbuf[pagesize] so free() not needed.

testcases/kernel/mem/mmapstress/mmapstress01.c int fileokay(char *file, uchar_t * expbuf) · line 662

Quoted 3 lines

testcases/kernel/mem/mmapstress/mmapstress01.c +1 -0

+			close(fd);
  			anyfail();

anyfail() calls tst_brkm() - the program terminates and closes fd.

Quoted 15 lines

testcases/kernel/mem/mmapstress/mmapstress01.c +2 -0

  			/*****	**	*****/
  			return 0;

@@ -668,6 +670,8 @@ int fileokay(char *file, uchar_t * expbuf)
  				(void)fprintf(stderr, "read %d of %ld bytes\n",
  					      (i * pagesize) + cnt,
  					      (long)mapsize);
+				free(readbuf);
+				close(fd);
  				return 0;
  			}
  		}

@@ -688,10 +692,13 @@ int fileokay(char *file, uchar_t * expbuf)
  					      "(fsize %ld)\n", i, j,
  					      statbuf.st_size);
  #endif /* LARGE_FILE */

testcases/kernel/mem/mmapstress/mmapstress01.c int fileokay(char *file, uchar_t * expbuf) · line 664

Quoted 3 lines

testcases/kernel/mem/mmapstress/mmapstress01.c +2 -0

+				free(readbuf);
+				close(fd);
  				return 0;

In both above cases it'll call anyfail() after checking fileokay()'s return value.
If it seems not obvious we can close 'fd' here and it'd be better to use SAFE_CLOSE() macro from safe_macros.h.

testcases/kernel/mem/mmapstress/mmapstress01.c int fileokay(char *file, uchar_t * expbuf) · line 704

Quoted 8 lines

testcases/kernel/mem/mmapstress/mmapstress01.c +1 -0

  			}
  		}
  	}
+	free(readbuf);
  	close(fd);
 
  	return 1;

Thanks,
Alexey

There's no cleanup function for SAFE_CLOSE(). so,
close() is the same as SAFE_CLOSE(NULL, fd).

Quoted 4 lines · Alexey Kodanev

In both above cases it'll call anyfail() after checking fileokay()'s return value.
If it seems not obvious we can close 'fd' here and it'd be better to use SAFE_CLOSE() macro from safe_macros.h.

There's no cleanup function for SAFE_CLOSE().
so,
close() is the same as SAFE_CLOSE(NULL, fd).

From: "Zeng Linggang" <[email protected]>
To: "Jiangang Wei" <[email protected]>
Cc: [email protected]
Sent: Tuesday, 19 May, 2015 5:34:01 AM
Subject: [LTP] [PATCH] mmapstress01: Modify readbuf's type and define it to uchar_t

readbuf is defined to char. That is different with the previous defined. If readbuf's value is more than 127, that would be regarded as negative number. And expbuf is uchar_t, the value is always nonegative. That will make the test fail because the different data type.

Signed-off-by Zeng Linggang <[email protected]>