[dpdk-dev] app/testpmd: fix argument cannot be negative
Checks
Commit Message
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 | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Comments
Hi Daniel,
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> Sent: Tuesday, July 25, 2017 12:00 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: fix argument cannot be negative
Sorry for the nit-pick review, but there should be some description
here in the commit message, describing what was fixed. I see it is
stated in the title, but a short paragraph stating the issue and
what wrong effect the patch fixes is very useful in git bisect :)
Comments on code below, -Harry
> 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 | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index ee6644d10..b77fb96e1 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -3292,7 +3292,7 @@ uint8_t *
> open_ddp_package_file(const char *file_path, uint32_t *size)
> {
> FILE *fh = fopen(file_path, "rb");
> - uint32_t pkg_size;
> + off_t pkg_size;
I don't see a mention of off_t anywhere else in this file,
perhaps use a commonly used datatype that is suitable
to hold the "long int" that ftell() returns, eg: int64_t ?
> uint8_t *buf = NULL;
> int ret = 0;
>
> @@ -3312,6 +3312,12 @@ open_ddp_package_file(const char *file_path, uint32_t *size)
> }
>
> pkg_size = ftell(fh);
> + if (pkg_size == -1) {
> + fclose(fh);
> + printf("%s: The stream specified is not a seekable stream\n"
> + , __func__);
> + return buf;
> + }
Given that malloc(0) is implementation defined, we should probably check
to ensure that pkg_size is > 0 before malloc(). Perhaps not explicitly
called out by coverity in this case, but worth fixing as that code is
being modified already.
>
> buf = (uint8_t *)malloc(pkg_size);
> if (!buf) {
> --
> 2.13.3
@@ -3292,7 +3292,7 @@ uint8_t *
open_ddp_package_file(const char *file_path, uint32_t *size)
{
FILE *fh = fopen(file_path, "rb");
- uint32_t pkg_size;
+ off_t pkg_size;
uint8_t *buf = NULL;
int ret = 0;
@@ -3312,6 +3312,12 @@ open_ddp_package_file(const char *file_path, uint32_t *size)
}
pkg_size = ftell(fh);
+ if (pkg_size == -1) {
+ fclose(fh);
+ printf("%s: The stream specified is not a seekable stream\n"
+ , __func__);
+ return buf;
+ }
buf = (uint8_t *)malloc(pkg_size);
if (!buf) {