[dpdk-dev] app/testpmd: wrong usage of fseek & ftell to determine filesize

Message ID 20170818141736.4769-1-danielx.t.mrzyglod@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Daniel Mrzyglod Aug. 18, 2017, 2:17 p.m. UTC
  This issue was about passing unsigned argument where should be signed number.
In reality this is about wrong usage of fseek and ftell to determine
filesize.
This patch is compliant to suggestions from FIO19-C:
"Do not use fseek() and ftell() to compute the size of a regular file"

Coverity issue: 143454
Fixes: a92a5a2cbbff ("app/testpmd: add command for loading DDP")

Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
 app/test-pmd/config.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)
  

Comments

Xing, Beilei Aug. 18, 2017, 4:18 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> Sent: Friday, August 18, 2017 10:18 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Mrzyglod, DanielX T <danielx.t.mrzyglod@intel.com>
> Subject: [dpdk-dev] [PATCH] app/testpmd: wrong usage of fseek & ftell to
> determine filesize
> 
> This issue was about passing unsigned argument where should be signed
> number.
> In reality this is about wrong usage of fseek and ftell to determine filesize.
> This patch is compliant to suggestions from FIO19-C:
> "Do not use fseek() and ftell() to compute the size of a regular file"
> 
> Coverity issue: 143454
> Fixes: a92a5a2cbbff ("app/testpmd: add command for loading DDP")
> 
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>

Reviewed-by: Beilei Xing <beilei.xing@intel.com>
  
Xing, Beilei Aug. 22, 2017, 2:03 a.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> Sent: Friday, August 18, 2017 10:18 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Mrzyglod, DanielX T <danielx.t.mrzyglod@intel.com>
> Subject: [dpdk-dev] [PATCH] app/testpmd: wrong usage of fseek & ftell to
> determine filesize
> 
> This issue was about passing unsigned argument where should be signed
> number.
> In reality this is about wrong usage of fseek and ftell to determine filesize.
> This patch is compliant to suggestions from FIO19-C:
> "Do not use fseek() and ftell() to compute the size of a regular file"
> 
> Coverity issue: 143454
> Fixes: a92a5a2cbbff ("app/testpmd: add command for loading DDP")
> 
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>

Acked-by: Beilei Xing <beilei.xing@intel.com>
  
Daniel Mrzyglod Oct. 4, 2017, 1:09 p.m. UTC | #3
>>
>> This issue was about passing unsigned argument where should be signed
>> number.
>> In reality this is about wrong usage of fseek and ftell to determine filesize.
>> This patch is compliant to suggestions from FIO19-C:
>> "Do not use fseek() and ftell() to compute the size of a regular file"
>>
>> Coverity issue: 143454
>> Fixes: a92a5a2cbbff ("app/testpmd: add command for loading DDP")
>>
>> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
>
>Acked-by: Beilei Xing <beilei.xing@intel.com>


Hi Jingjing
I would like to ask for a feedback about status of proposed fix.
Will it go for integration or does it need any other additional work.

Best regards
Daniel
  
Ferruh Yigit Oct. 9, 2017, 4:21 a.m. UTC | #4
On 8/22/2017 3:03 AM, Xing, Beilei wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
>> Sent: Friday, August 18, 2017 10:18 PM
>> To: Xing, Beilei <beilei.xing@intel.com>
>> Cc: dev@dpdk.org; Mrzyglod, DanielX T <danielx.t.mrzyglod@intel.com>
>> Subject: [dpdk-dev] [PATCH] app/testpmd: wrong usage of fseek & ftell to
>> determine filesize
>>
>> This issue was about passing unsigned argument where should be signed
>> number.
>> In reality this is about wrong usage of fseek and ftell to determine filesize.
>> This patch is compliant to suggestions from FIO19-C:
>> "Do not use fseek() and ftell() to compute the size of a regular file"
>>
>> Coverity issue: 143454
>> Fixes: a92a5a2cbbff ("app/testpmd: add command for loading DDP")
>>
>> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> 
> Acked-by: Beilei Xing <beilei.xing@intel.com>

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 3ae3e1cd8..32b0c1566 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -40,6 +40,10 @@ 
 #include <inttypes.h>
 
 #include <sys/queue.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include <rte_common.h>
 #include <rte_byteorder.h>
@@ -3295,46 +3299,43 @@  port_dcb_info_display(uint8_t port_id)
 uint8_t *
 open_ddp_package_file(const char *file_path, uint32_t *size)
 {
-	FILE *fh = fopen(file_path, "rb");
-	uint32_t pkg_size;
+	int fd = open(file_path, O_RDONLY);
+	off_t pkg_size;
 	uint8_t *buf = NULL;
 	int ret = 0;
+	struct stat st_buf;
 
 	if (size)
 		*size = 0;
 
-	if (fh == NULL) {
+	if (fd == -1) {
 		printf("%s: Failed to open %s\n", __func__, file_path);
 		return buf;
 	}
 
-	ret = fseek(fh, 0, SEEK_END);
-	if (ret < 0) {
-		fclose(fh);
+	if ((fstat(fd, &st_buf) != 0) || (!S_ISREG(st_buf.st_mode))) {
+		close(fd);
 		printf("%s: File operations failed\n", __func__);
 		return buf;
 	}
 
-	pkg_size = ftell(fh);
+	pkg_size = st_buf.st_size;
+	if (pkg_size < 0) {
+		close(fd);
+		printf("%s: File operations failed\n", __func__);
+		return buf;
+	}
 
 	buf = (uint8_t *)malloc(pkg_size);
 	if (!buf) {
-		fclose(fh);
+		close(fd);
 		printf("%s: Failed to malloc memory\n",	__func__);
 		return buf;
 	}
 
-	ret = fseek(fh, 0, SEEK_SET);
+	ret = read(fd, buf, pkg_size);
 	if (ret < 0) {
-		fclose(fh);
-		printf("%s: File seek operation failed\n", __func__);
-		close_ddp_package_file(buf);
-		return NULL;
-	}
-
-	ret = fread(buf, 1, pkg_size, fh);
-	if (ret < 0) {
-		fclose(fh);
+		close(fd);
 		printf("%s: File read operation failed\n", __func__);
 		close_ddp_package_file(buf);
 		return NULL;
@@ -3343,7 +3344,7 @@  open_ddp_package_file(const char *file_path, uint32_t *size)
 	if (size)
 		*size = pkg_size;
 
-	fclose(fh);
+	close(fd);
 
 	return buf;
 }