DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Hunt <david.hunt@intel.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Bruce Richardson <bruce.richardson@intel.com>,
	"Thomas Monjalon" <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v4 2/2] eal: add additional info if core mask too long
Date: Thu, 23 Sep 2021 11:21:32 +0100	[thread overview]
Message-ID: <b6716fec-407f-c286-6489-2f459c7d138b@intel.com> (raw)
In-Reply-To: <CAJFAV8zU+R4k0nnXit2jjpTN--bUdndH9MPOJu7miSBxsKUnDA@mail.gmail.com>


On 23/9/2021 9:12 AM, David Marchand wrote:
> On Wed, Sep 22, 2021 at 2:29 PM David Hunt <david.hunt@intel.com> wrote:
>>
>>          int i, j, idx;
>>          int val;
>>          char c;
>> +       int lcores[RTE_MAX_LCORE];
>> +       char *coremask_copy = NULL;
> Contrary to patch 1, coremask_copy is used.
> But, coremask is const, we can directly use it in log messages.
>
> So same as patch 1, please remove coremask_copy.


const protects the data being pointed at from being modified, but the 
pointer itself can still be modified, and the pointer is indeed 
incremented to cut off leading zeroes and spaces.

For example, if I provide "  0x0" as a parameter,coremask prints as "0", 
where as coremask_copy prints as "  0x0".

Similarly, if I provide "0x" as a parameter,coremask prints as "", where 
as coremask_copy prints as "  0x".

Instead of using strdup, I can use "const char *coremask_orig = 
coremask;", and remove the strdup and the frees...


>
>>          for (idx = 0; idx < RTE_MAX_LCORE; idx++)
>>                  cores[idx] = -1;
>>          idx = 0;
>>
>> +       coremask_copy = strdup(coremask);
>> +       if (coremask_copy == NULL) {
>> +               RTE_LOG(ERR, EAL, "Unable to duplicate coremask\n");
>> +               return -ENOMEM;
>> +       }
>> +
>>          /* Remove all blank characters ahead and after .
>>           * Remove 0x/0X if exists.
>>           */
>> @@ -767,30 +775,64 @@ eal_parse_coremask(const char *coremask, int *cores)
>>          i = strlen(coremask);
>>          while ((i > 0) && isblank(coremask[i - 1]))
>>                  i--;
>> -       if (i == 0)
>> -               return -1;
>> +       if (i == 0) {
>> +               RTE_LOG(ERR, EAL, "No lcores in coremask: %s\n", coremask_copy);
>> +               goto err;
>> +       }
>>
>> -       for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
>> +       for (i = i - 1; i >= 0; i--) {
>>                  c = coremask[i];
>>                  if (isxdigit(c) == 0) {
>>                          /* invalid characters */
>> -                       return -1;
>> +                       RTE_LOG(ERR, EAL, "invalid characters in coremask: %s\n",
>> +                                       coremask_copy);
>> +                       goto err;
>>                  }
>>                  val = xdigit2val(c);
>> -               for (j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; j++, idx++)
>> +               for (j = 0; j < BITS_PER_HEX; j++, idx++)
>>                  {
>>                          if ((1 << j) & val) {
>> -                               cores[idx] = count;
>> -                               count++;
>> +                               if (count < RTE_MAX_LCORE) {
>> +                                       lcores[count++] = idx;
>> +                               } else {
>> +                                       RTE_LOG(ERR, EAL, "Too many lcores provided. Cannot exceed %d\n",
>> +                                                       RTE_MAX_LCORE);
>> +                                       goto err;
>> +                               }
> You can invert those blocks:
>
> if (count >= RTE_MAX_LCORE) {
>     RTE_LOG();
>     goto err;
> }
>
> lcores[count] = idx;
> count++;
>

+1


>
>>                          }
>>                  }
>>          }
>>          for (; i >= 0; i--)
>> -               if (coremask[i] != '0')
>> -                       return -1;
>> -       if (count == 0)
>> -               return -1;
>> +               if (coremask[i] != '0') {
>> +                       RTE_LOG(ERR, EAL, "invalid characters in coremask: %s\n",
>> +                                       coremask_copy);
>> +                       goto err;
>> +               }
>> +       if (count == 0) {
>> +               RTE_LOG(ERR, EAL, "No lcores in coremask: %s\n", coremask_copy);
>> +               goto err;
>> +       }
>> +
>> +       if (check_core_list(lcores, count))
>> +               goto err;
>> +
>> +       /*
>> +        * Now that we've gto a list of cores no longer than
>> +        * RTE_MAX_LCORE, and no lcore in that list is greater
>> +        * than RTE_MAX_LCORE, populate the cores
>> +        * array and return.
>> +        */
>> +       for (k = 0; k < count; k++)
>> +               cores[lcores[k]] = k;
>> +
>> +       if (coremask_copy)
>> +               free(coremask_copy);
>> +
>>          return 0;
>> +err:
>> +       if (coremask_copy)
>> +               free(coremask_copy);
>> +       return -1;
>>   }
>>
>>   static int
>> --
>> 2.17.1
>>
>
>

  reply	other threads:[~2021-09-23 10:21 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-09 13:45 [dpdk-dev] build: Increase the default value of RTE_MAX_LCORE David Hunt
2021-09-09 13:45 ` [dpdk-dev] [PATCH v1 1/6] build: increase default of max lcores to 512 David Hunt
2021-09-09 14:37   ` Bruce Richardson
2021-09-10  6:51     ` David Marchand
2021-09-10  7:54       ` Bruce Richardson
2021-09-10  8:06         ` David Marchand
2021-09-10  8:24           ` Thomas Monjalon
2021-09-14  9:34             ` David Hunt
2021-09-14 10:00               ` David Marchand
2021-09-14 11:07                 ` David Hunt
2021-09-14 11:29                   ` David Marchand
2021-09-15 12:13                     ` David Hunt
2021-11-17 15:55                   ` Morten Brørup
2021-11-17 19:01                     ` David Hunt
2021-09-15 12:11   ` [dpdk-dev] [PATCH v2] eal: add additional info if lcore exceeds max cores David Hunt
2021-09-16 12:34     ` David Marchand
2021-09-20  9:30       ` David Hunt
2021-09-21 11:50     ` [dpdk-dev] [PATCH v3 1/2] eal: add additional info if core list too long David Hunt
2021-09-21 11:50       ` [dpdk-dev] [PATCH v3 2/2] eal: add additional info if core mask " David Hunt
2021-09-21 12:00         ` Bruce Richardson
2021-09-21 11:57       ` [dpdk-dev] [PATCH v3 1/2] eal: add additional info if core list " Bruce Richardson
2021-09-21 12:04         ` David Hunt
2021-09-21 13:16           ` David Hunt
2021-09-21 13:20             ` Bruce Richardson
2021-09-21 13:51       ` David Marchand
2021-09-21 15:10         ` David Hunt
2021-09-22 12:29       ` [dpdk-dev] [PATCH v4 " David Hunt
2021-09-22 12:29         ` [dpdk-dev] [PATCH v4 2/2] eal: add additional info if core mask " David Hunt
2021-09-23  8:12           ` David Marchand
2021-09-23 10:21             ` David Hunt [this message]
2021-09-23  8:11         ` [dpdk-dev] [PATCH v4 1/2] eal: add additional info if core list " David Marchand
2021-09-23  9:47           ` David Hunt
2021-09-23 11:02         ` [dpdk-dev] [PATCH v5 " David Hunt
2021-09-23 11:02           ` [dpdk-dev] [PATCH v5 2/2] eal: add additional info if core mask " David Hunt
2021-11-02 17:45             ` David Marchand
2021-11-03 10:27               ` David Hunt
2021-11-03 10:29                 ` David Marchand
2021-11-03 13:30                 ` David Hunt
2021-11-03 14:32           ` [dpdk-dev] [PATCH v6 1/2] eal: add additional info if core list " David Hunt
2021-11-03 14:32             ` [dpdk-dev] [PATCH v6 2/2] eal: add additional info if core mask " David Hunt
2021-11-05 10:50               ` David Marchand
2021-09-09 13:45 ` [dpdk-dev] [PATCH v1 2/6] lib/power: reduce memory footprint of acpi lib David Hunt
2021-09-09 13:45 ` [dpdk-dev] [PATCH v1 3/6] lib/power: reduce memory footprint of pstate lib David Hunt
2021-09-09 13:45 ` [dpdk-dev] [PATCH v1 4/6] lib/power: reduce memory footprint of cppc lib David Hunt
2021-09-09 13:45 ` [dpdk-dev] [PATCH v1 5/6] lib/power: reduce memory footprint of channels David Hunt
2021-09-09 13:45 ` [dpdk-dev] [PATCH v1 6/6] lib/power: switch empty poll to max cores config David Hunt

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=b6716fec-407f-c286-6489-2f459c7d138b@intel.com \
    --to=david.hunt@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).