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

Message ID 1422610133-20756-2-git-send-email-ola.liljedahl@linaro.org
State New
Headers show

Commit Message

Ola Liljedahl Jan. 30, 2015, 9:28 a.m.
odp_cpumask_to_str() definition updated to return number of chars written
(on success) and return <0 on failure.
Added ODP_CPUMASK_BUFSIZE define with recommended output buffer size.
Updated the implementation and usages of this call to match definition and
recommended usage.
Doxygen documentation rephrasing in odp_cpumask.h.

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 +-
 platform/linux-generic/include/api/odp_cpumask.h | 102 ++++++++++++++---------
 platform/linux-generic/odp_cpumask.c             |  30 ++++---
 test/api_test/odp_common.c                       |   4 +-
 test/performance/odp_scheduling.c                |   4 +-
 9 files changed, 94 insertions(+), 66 deletions(-)

Patch

diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
index 492664e..9a8700f 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 f2d2fc7..9f20925 100644
--- a/example/ipsec/odp_ipsec.c
+++ b/example/ipsec/odp_ipsec.c
@@ -1175,7 +1175,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 */
@@ -1224,7 +1224,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 18403da..7a2db39 100644
--- a/example/l2fwd/odp_l2fwd.c
+++ b/example/l2fwd/odp_l2fwd.c
@@ -292,7 +292,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 */
@@ -334,7 +334,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 c4c720b..be6b1b5 100644
--- a/example/packet/odp_pktio.c
+++ b/example/packet/odp_pktio.c
@@ -285,7 +285,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));
@@ -322,7 +322,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 fafa0a4..ea351b2 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/platform/linux-generic/include/api/odp_cpumask.h b/platform/linux-generic/include/api/odp_cpumask.h
index b746a4d..abeeeed 100644
--- a/platform/linux-generic/include/api/odp_cpumask.h
+++ b/platform/linux-generic/include/api/odp_cpumask.h
@@ -20,8 +20,10 @@  extern "C" {
 
 #include <string.h>
 #include <sched.h>
+#include <sys/types.h>
 
 #include <odp_std_types.h>
+#include <odp_config.h>
 
 /** @addtogroup odp_scheduler
  *  CPU mask operations.
@@ -29,6 +31,11 @@  extern "C" {
  */
 
 /**
+ * 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.
@@ -47,111 +54,125 @@  typedef struct odp_cpumask_t {
 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
- * @param mask	CPU mask to flush with zero value
+ * Clear entire CPU mask
+ * @param mask	CPU mask to update
  */
 void odp_cpumask_zero(odp_cpumask_t *mask);
 
 /**
- * Add cpu to mask
- * @param mask  add cpu number in CPU mask
+ * Add CPU to mask
+ * @param mask  CPU mask to update
  * @param cpu   CPU number
  */
 void odp_cpumask_set(odp_cpumask_t *mask, int cpu);
 
 /**
- * Remove cpu from mask
- * @param mask  clear cpu number from CPU mask
+ * Remove CPU from mask
+ * @param mask  CPU mask to update
  * @param cpu   CPU number
  */
 void odp_cpumask_clr(odp_cpumask_t *mask, int cpu);
 
 /**
- * Test if cpu is a member of mask
- * @param mask  CPU mask to check if cpu num set or not
+ * Test if CPU is a member of mask
+ * @param mask  CPU mask to test
  * @param cpu   CPU number
- * @return      non-zero if set otherwise 0
+ * @return      non-zero if set
+ * @retval	0 if not set
  */
 int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu);
 
 /**
- * Count number of cpus in mask
+ * Count number of CPU's in mask
  * @param mask  CPU mask
- * @return cpumask count
+ * @return population count
  */
 int odp_cpumask_count(const odp_cpumask_t *mask);
 
 /**
- * Logical AND over two source masks.
+ * Member-wise AND over two CPU masks
  *
- * @param dest    Destination mask, can be one of the source masks
- * @param src1    Source mask 1
- * @param src2    Source mask 2
+ * @param dest    Destination CPU mask (may be one of the source masks)
+ * @param src1    Source CPU mask 1
+ * @param src2    Source CPU mask 2
  */
 void odp_cpumask_and(odp_cpumask_t *dest, odp_cpumask_t *src1,
 		     odp_cpumask_t *src2);
 
 /**
- * Logical OR over two source masks.
+ * Member-wise OR over two CPU masks
  *
- * @param dest    Destination mask, can be one of the source masks
- * @param src1    Source mask 1
- * @param src2    Source mask 2
+ * @param dest    Destination CPU mask (may be one of the source masks)
+ * @param src1    Source CPU mask 1
+ * @param src2    Source CPU mask 2
  */
 void odp_cpumask_or(odp_cpumask_t *dest, odp_cpumask_t *src1,
 		    odp_cpumask_t *src2);
 
 /**
- * Logical XOR over two source masks.
+ * Member-wise XOR over two CPU masks
  *
- * @param dest    Destination mask, can be one of the source masks
- * @param src1    Source mask 1
- * @param src2    Source mask 2
+ * @param dest    Destination CPU mask (may be one of the source masks)
+ * @param src1    Source CPU mask 1
+ * @param src2    Source CPU mask 2
  */
 void odp_cpumask_xor(odp_cpumask_t *dest, odp_cpumask_t *src1,
 		     odp_cpumask_t *src2);
 
 /**
- * Test if two masks contain the same cpus
+ * Test if two CPU masks contain the same CPU's
+ *
+ * @param mask1    CPU mask 1
+ * @param mask2    CPU mask 2
  */
 int odp_cpumask_equal(const odp_cpumask_t *mask1,
 		      const odp_cpumask_t *mask2);
 
 /**
  * Copy a CPU mask
+ *
+ * @param dest    Destination CPU mask
+ * @param src     Source CPU mask
  */
 void odp_cpumask_copy(odp_cpumask_t *dest, const odp_cpumask_t *src);
 
 /**
- * Find first CPU that is set in the mask
+ * Find first set CPU in mask
  *
- * @param mask is the mask to be searched
- * @return cpu else -1 if no bits set in cpumask
+ * @param mask    CPU mask
+ *
+ * @return cpu number
+ * @retval <0 if no CPU found
  */
 int odp_cpumask_first(const odp_cpumask_t *mask);
 
 /**
- * Find last CPU that is set in the mask
+ * Find last set CPU in mask
+ *
+ * @param mask    CPU mask
  *
- * @param mask is the mask to be searched
- * @return cpu else -1 if no bits set in cpumask
+ * @return cpu number
+ * @retval <0 if no CPU found
  */
 int odp_cpumask_last(const odp_cpumask_t *mask);
 
 /**
- * Find next cpu in mask
+ * Find next set CPU in mask
  *
- * Finds the next cpu in the CPU mask, starting at the cpu passed.
- * Use with odp_cpumask_first to traverse a CPU mask, i.e.
+ * Finds the next CPU in the CPU mask, starting at the CPU passed.
+ * Use with odp_cpumask_first to traverse a CPU mask, e.g.
  *
  * int cpu = odp_cpumask_first(&mask);
  * while (0 <= cpu) {
@@ -160,9 +181,10 @@  int odp_cpumask_last(const odp_cpumask_t *mask);
  *     cpu = odp_cpumask_next(&mask, cpu);
  * }
  *
- * @param mask        CPU mask to find next cpu in
+ * @param mask        CPU mask
  * @param cpu         CPU to start from
- * @return cpu found else -1
+ * @return cpu number
+ * @retval <0 if no CPU found
  *
  * @see odp_cpumask_first()
  */
diff --git a/platform/linux-generic/odp_cpumask.c b/platform/linux-generic/odp_cpumask.c
index d9931b4..51e7b45 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 bce6f09..b5c1a34 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 32155bd..6717189 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));