From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: "Mrzyglod, DanielX T" <danielx.t.mrzyglod@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Xing, Beilei" <beilei.xing@intel.com>
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix argument cannot be negative
Date: Tue, 25 Jul 2017 12:16:14 +0000 [thread overview]
Message-ID: <E923DB57A917B54B9182A2E928D00FA640C3FF5B@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <20170725105954.82081-1-danielx.t.mrzyglod@intel.com>
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
prev parent reply other threads:[~2017-07-25 12:16 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-25 10:59 Daniel Mrzyglod
2017-07-25 12:16 ` Van Haaren, Harry [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E923DB57A917B54B9182A2E928D00FA640C3FF5B@IRSMSX102.ger.corp.intel.com \
--to=harry.van.haaren@intel.com \
--cc=beilei.xing@intel.com \
--cc=danielx.t.mrzyglod@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).