Hi Daniel,
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Daniel Mrzyglod > Sent: Tuesday, July 25, 2017 12:00 PM > To: Xing, Beilei <beilei.x...@intel.com> > Cc: dev@dpdk.org; Mrzyglod, DanielX T <danielx.t.mrzyg...@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.mrzyg...@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
