[PATCHv3,01/18] api: odp_cpumask.h: odp_cpumask_to_str() return chars written or error

Add define ODP_CPUMASK_BUFSIZE for minimum output buffer size for
odp_cpumask_to_str().
odp_cpumask_to_str() takes output buffer size as input and returns number
of chars written (on success), <0 on failure.
Updated the implementation.
Updated all usages in example and test programs.


Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
---
(This document/code contribution attached is provided under the terms of
agreement LES-LTM-21309)

 example/generator/odp_generator.c                  |  4 +--
 example/ipsec/odp_ipsec.c                          |  4 +--
 example/l2fwd/odp_l2fwd.c                          |  4 +--
 example/packet/odp_pktio.c                         |  4 +--
 example/timer/odp_timer_test.c                     |  4 +--
 include/odp/api/cpumask.h                          | 23 ++++++++++++-----
 .../linux-generic/include/odp/plat/cpumask_types.h |  5 ++++
 platform/linux-generic/odp_cpumask.c               | 30 +++++++++++++---------
 test/api_test/odp_common.c                         |  4 +--
 test/performance/odp_scheduling.c                  |  4 +--
 10 files changed, 54 insertions(+), 32 deletions(-)

Checkpatch flags this:

WARNING: braces {} are not necessary for single statement blocks
#234: FILE: platform/linux-generic/odp_cpumask.c:73:
+ if (len < 4) {
+ return -1; /* Failure */
+ }

WARNING: braces {} are not necessary for single statement blocks
#255: FILE: platform/linux-generic/odp_cpumask.c:88:
+ if (len < (3 + nibbles)) {
+ return -1; /* Failure */
+ }

total: 0 errors, 2 warnings, 0 checks, 224 lines checked



On Mon, Feb 2, 2015 at 9:42 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> Add define ODP_CPUMASK_BUFSIZE for minimum output buffer size for
> odp_cpumask_to_str().
> odp_cpumask_to_str() takes output buffer size as input and returns number
> of chars written (on success), <0 on failure.
> Updated the implementation.
> Updated all usages in example and test programs.
>
>
> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
>
>  example/generator/odp_generator.c                  |  4 +--
>  example/ipsec/odp_ipsec.c                          |  4 +--
>  example/l2fwd/odp_l2fwd.c                          |  4 +--
>  example/packet/odp_pktio.c                         |  4 +--
>  example/timer/odp_timer_test.c                     |  4 +--
>  include/odp/api/cpumask.h                          | 23 ++++++++++++-----
>  .../linux-generic/include/odp/plat/cpumask_types.h |  5 ++++
>  platform/linux-generic/odp_cpumask.c               | 30
> +++++++++++++---------
>  test/api_test/odp_common.c                         |  4 +--
>  test/performance/odp_scheduling.c                  |  4 +--
>  10 files changed, 54 insertions(+), 32 deletions(-)
>
> diff --git a/example/generator/odp_generator.c
> b/example/generator/odp_generator.c
> index 03dab50..2433cb6 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -545,7 +545,7 @@ int main(int argc, char *argv[])
>         int i;
>         odp_shm_t shm;
>         odp_cpumask_t cpumask;
> -       char cpumaskstr[64];
> +       char cpumaskstr[ODP_CPUMASK_BUFSIZE];
>         odp_pool_param_t params;
>
>         /* Init ODP before calling anything else */
> @@ -596,7 +596,7 @@ int main(int argc, char *argv[])
>          * Start mapping thread from CPU #1
>          */
>         num_workers = odph_linux_cpumask_default(&cpumask, num_workers);
> -       odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> +       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
> index 03f2419..869fd3a 100644
> --- a/example/ipsec/odp_ipsec.c
> +++ b/example/ipsec/odp_ipsec.c
> @@ -1193,7 +1193,7 @@ main(int argc, char *argv[])
>         int stream_count;
>         odp_shm_t shm;
>         odp_cpumask_t cpumask;
> -       char cpumaskstr[64];
> +       char cpumaskstr[ODP_CPUMASK_BUFSIZE];
>         odp_pool_param_t params;
>
>         /* Init ODP before calling anything else */
> @@ -1242,7 +1242,7 @@ main(int argc, char *argv[])
>          * Start mapping thread from CPU #1
>          */
>         num_workers = odph_linux_cpumask_default(&cpumask, num_workers);
> -       odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> +       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
> index 7a520fb..e22ec0c 100644
> --- a/example/l2fwd/odp_l2fwd.c
> +++ b/example/l2fwd/odp_l2fwd.c
> @@ -294,7 +294,7 @@ int main(int argc, char *argv[])
>         int num_workers;
>         odp_shm_t shm;
>         odp_cpumask_t cpumask;
> -       char cpumaskstr[64];
> +       char cpumaskstr[ODP_CPUMASK_BUFSIZE];
>         odp_pool_param_t params;
>
>         /* Init ODP before calling anything else */
> @@ -336,7 +336,7 @@ int main(int argc, char *argv[])
>          * Start mapping thread from CPU #1
>          */
>         num_workers = odph_linux_cpumask_default(&cpumask, num_workers);
> -       odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> +       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
> index 28ed98c..e38029a 100644
> --- a/example/packet/odp_pktio.c
> +++ b/example/packet/odp_pktio.c
> @@ -287,7 +287,7 @@ int main(int argc, char *argv[])
>         int i;
>         int cpu;
>         odp_cpumask_t cpumask;
> -       char cpumaskstr[64];
> +       char cpumaskstr[ODP_CPUMASK_BUFSIZE];
>         odp_pool_param_t params;
>
>         args = calloc(1, sizeof(args_t));
> @@ -324,7 +324,7 @@ int main(int argc, char *argv[])
>          * Start mapping thread from CPU #1
>          */
>         num_workers = odph_linux_cpumask_default(&cpumask, num_workers);
> -       odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> +       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> diff --git a/example/timer/odp_timer_test.c
> b/example/timer/odp_timer_test.c
> index 0df041f..299ed89 100644
> --- a/example/timer/odp_timer_test.c
> +++ b/example/timer/odp_timer_test.c
> @@ -317,7 +317,7 @@ int main(int argc, char *argv[])
>         odp_timer_pool_param_t tparams;
>         odp_timer_pool_info_t tpinfo;
>         odp_cpumask_t cpumask;
> -       char cpumaskstr[64];
> +       char cpumaskstr[ODP_CPUMASK_BUFSIZE];
>
>         printf("\nODP timer example starts\n");
>
> @@ -358,7 +358,7 @@ int main(int argc, char *argv[])
>          * Start mapping thread from CPU #1
>          */
>         num_workers = odph_linux_cpumask_default(&cpumask, num_workers);
> -       odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> +       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h
> index 7482899..65969c3 100644
> --- a/include/odp/api/cpumask.h
> +++ b/include/odp/api/cpumask.h
> @@ -18,12 +18,20 @@
>  extern "C" {
>  #endif
>
> +#include <sys/types.h>
> +#include <odp/config.h>
> +
>  /** @addtogroup odp_scheduler
>   *  CPU mask operations.
>   *  @{
>   */
>
> - /**
> +/**
> + * @def ODP_CPUMASK_BUFSIZE
> + * Minimum size of output buffer for odp_cpumask_to_str()
> + */
> +
> +/**
>   * Add CPU mask bits from a string
>   *
>   * @param mask   CPU mask to modify
> @@ -33,13 +41,16 @@ extern "C" {
>  void odp_cpumask_from_str(odp_cpumask_t *mask, const char *str);
>
>  /**
> - * Write CPU mask as a string of hexadecimal digits
> + * Format CPU mask as a string of hexadecimal digits
> + *
> + * @param mask CPU mask to format
> + * @param[out] buf Output buffer (use ODP_CPUMASK_BUFSIZE)
> + * @param bufsz Size of output buffer
>   *
> - * @param mask   CPU mask
> - * @param str    String for output
> - * @param len    Size of string length (incl. ending zero)
> + * @return number of characters written (including terminating null char)
> + * @retval <0 on failure (buffer too small)
>   */
> -void odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int len);
> +ssize_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *buf, ssize_t
> bufsz);
>
>  /**
>   * Clear entire mask
> diff --git a/platform/linux-generic/include/odp/plat/cpumask_types.h
> b/platform/linux-generic/include/odp/plat/cpumask_types.h
> index 6d9e129..20626f0 100644
> --- a/platform/linux-generic/include/odp/plat/cpumask_types.h
> +++ b/platform/linux-generic/include/odp/plat/cpumask_types.h
> @@ -25,6 +25,11 @@ extern "C" {
>  #include <odp/std_types.h>
>
>  /**
> + * Minimum size of output buffer for odp_cpumask_to_str()
> + */
> +#define ODP_CPUMASK_BUFSIZE ((ODP_CONFIG_MAX_THREADS + 3) / 4 + 3)
> +
> +/**
>   * CPU mask
>   *
>   * Don't access directly, use access functions.
> diff --git a/platform/linux-generic/odp_cpumask.c
> b/platform/linux-generic/odp_cpumask.c
> index 6088ca2..6c27e8b 100644
> --- a/platform/linux-generic/odp_cpumask.c
> +++ b/platform/linux-generic/odp_cpumask.c
> @@ -8,6 +8,7 @@
>  #define _GNU_SOURCE
>  #endif
>  #include <sched.h>
> +#include <sys/types.h>
>
>  #include <odp/cpumask.h>
>  #include <odp_debug_internal.h>
> @@ -60,29 +61,33 @@ void odp_cpumask_from_str(odp_cpumask_t *mask, const
> char *str_in)
>         memcpy(&mask->set, &cpuset, sizeof(cpuset));
>  }
>
> -void odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, int len)
> +ssize_t odp_cpumask_to_str(const odp_cpumask_t *mask, char *str, ssize_t
> len)
>  {
>         char *p = str;
>         int cpu = odp_cpumask_last(mask);
> -       int nibbles;
> +       unsigned int nibbles;
>         int value;
>
> -       /* Quickly handle bad string length or empty mask */
> -       if (len <= 0)
> -               return;
> -       *str = 0;
> +       /* Handle bad string length, need at least 4 chars for "0x0" and
> +        * terminating null char */
> +       if (len < 4) {
> +               return -1; /* Failure */
> +       }
> +
> +       /* Handle no CPU found */
>         if (cpu < 0) {
> -               if (len >= 4)
> -                       strcpy(str, "0x0");
> -               return;
> +               strcpy(str, "0x0");
> +               return strlen(str) + 1; /* Success */
>         }
> +       /* CPU was found and cpu >= 0 */
>
> -       /* Compute number nibbles in cpumask that have bits set */
> +       /* Compute number of nibbles in cpumask that have bits set */
>         nibbles = (cpu / 4) + 1;
>
>         /* Verify minimum space (account for "0x" and termination) */
> -       if (len < (3 + nibbles))
> -               return;
> +       if (len < (3 + nibbles)) {
> +               return -1; /* Failure */
> +       }
>
>         /* Prefix */
>         *p++ = '0';
> @@ -110,6 +115,7 @@ void odp_cpumask_to_str(const odp_cpumask_t *mask,
> char *str, int len)
>
>         /* Terminate the string */
>         *p++ = 0;
> +       return p - str; /* Success */
>  }
>
>  void odp_cpumask_zero(odp_cpumask_t *mask)
> diff --git a/test/api_test/odp_common.c b/test/api_test/odp_common.c
> index 5f9504c..b5f2d4d 100644
> --- a/test/api_test/odp_common.c
> +++ b/test/api_test/odp_common.c
> @@ -30,14 +30,14 @@ __thread test_shared_data_t *test_shared_data;
>   /**< pointer to shared data *
>  void odp_print_system_info(void)
>  {
>         odp_cpumask_t cpumask;
> -       char str[32];
> +       char str[ODP_CPUMASK_BUFSIZE];
>
>         memset(str, 1, sizeof(str));
>
>         odp_cpumask_zero(&cpumask);
>
>         odp_cpumask_from_str(&cpumask, "0x1");
> -       odp_cpumask_to_str(&cpumask, str, sizeof(str));
> +       (void)odp_cpumask_to_str(&cpumask, str, sizeof(str));
>
>         printf("\n");
>         printf("ODP system info\n");
> diff --git a/test/performance/odp_scheduling.c
> b/test/performance/odp_scheduling.c
> index 3d7167f..ba13d8e 100644
> --- a/test/performance/odp_scheduling.c
> +++ b/test/performance/odp_scheduling.c
> @@ -828,7 +828,7 @@ int main(int argc, char *argv[])
>         int prios;
>         odp_shm_t shm;
>         test_globals_t *globals;
> -       char cpumaskstr[64];
> +       char cpumaskstr[ODP_CPUMASK_BUFSIZE];
>         odp_pool_param_t params;
>
>         printf("\nODP example starts\n\n");
> @@ -879,7 +879,7 @@ int main(int argc, char *argv[])
>          * Start mapping thread from CPU #1
>          */
>         num_workers = odph_linux_cpumask_default(&cpumask, num_workers);
> -       odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
> +       (void)odp_cpumask_to_str(&cpumask, cpumaskstr, sizeof(cpumaskstr));
>
>         printf("num worker threads: %i\n", num_workers);
>         printf("first CPU:          %i\n", odp_cpumask_first(&cpumask));
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>