[v2] eal: remove variable length array

Message ID 20181214204042.6435-1-jeffrey.b.shaw@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] eal: remove variable length array |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Jeff Shaw Dec. 14, 2018, 8:40 p.m. UTC
  Compilers that do not support the C99 standard, or do not implement
gcc extensions, may not support variable length arrays.

The code prior to this commit produced the following warning when
compiled with "-Wvla -std=c90".

  warning: ISO C90 forbids variable length array ‘array’ [-Wvla]

This commit removes the variable length array from the PMD debug
trace function by allocating memory dynamically on the stack using
alloca().

Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
---

V2:
 - Reference C99 in commit message instead of C11.
 - Remove unnecessary cast of alloca() returning void *.

---
 lib/librte_eal/common/include/rte_dev.h | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
  

Comments

Wiles, Keith Dec. 15, 2018, 2:26 p.m. UTC | #1
> On Dec 14, 2018, at 2:40 PM, Jeff Shaw <jeffrey.b.shaw@intel.com> wrote:
> 
> Compilers that do not support the C99 standard, or do not implement
> gcc extensions, may not support variable length arrays.
> 
> The code prior to this commit produced the following warning when
> compiled with "-Wvla -std=c90".
> 
>  warning: ISO C90 forbids variable length array ‘array’ [-Wvla]
> 
> This commit removes the variable length array from the PMD debug
> trace function by allocating memory dynamically on the stack using
> alloca().
> 
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> ---
> 
> V2:
> - Reference C99 in commit message instead of C11.
> - Remove unnecessary cast of alloca() returning void *.
> 
> ---
> lib/librte_eal/common/include/rte_dev.h | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index a9724dc91..6d7a220b0 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -47,22 +47,21 @@ __attribute__((format(printf, 2, 0)))
> static inline void
> rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
> {
> +	char *buffer;
> +	int buf_len;
> 	va_list ap;
> 
> 	va_start(ap, fmt);
> +	buf_len = vsnprintf(NULL, 0, fmt, ap) + 1;
> +	va_end(ap);
> 
> -	{
> -		char buffer[vsnprintf(NULL, 0, fmt, ap) + 1];
> +	buffer = alloca(buf_len);

Looks like some formatting problems with tab and/or spaces. DPDK is tabs of size 8 and no spaces before tabs.

The code looks fine to me, except for the calling vsnprintf twice. Could we have used char buffer[SOME_NUMBER_THAT_IS_REASONABLE];

This would remove the two calls to vsnprintf() and alloca usage.

If everyone is OK with it then I am ok after the formatting problems get fixed.
> 
> -		va_end(ap);
> -
> -		va_start(ap, fmt);
> -		vsnprintf(buffer, sizeof(buffer), fmt, ap);
> -		va_end(ap);
> +	va_start(ap, fmt);
> +	vsnprintf(buffer, buf_len, fmt, ap);
> +	va_end(ap);
> 
> -		rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s",
> -			func_name, buffer);
> -	}
> +	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s", func_name, buffer);
> }
> 
> /*
> -- 
> 2.14.3
> 

Regards,
Keith
  

Patch

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index a9724dc91..6d7a220b0 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -47,22 +47,21 @@  __attribute__((format(printf, 2, 0)))
 static inline void
 rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
 {
+	char *buffer;
+	int buf_len;
 	va_list ap;
 
 	va_start(ap, fmt);
+	buf_len = vsnprintf(NULL, 0, fmt, ap) + 1;
+	va_end(ap);
 
-	{
-		char buffer[vsnprintf(NULL, 0, fmt, ap) + 1];
+	buffer = alloca(buf_len);
 
-		va_end(ap);
-
-		va_start(ap, fmt);
-		vsnprintf(buffer, sizeof(buffer), fmt, ap);
-		va_end(ap);
+	va_start(ap, fmt);
+	vsnprintf(buffer, buf_len, fmt, ap);
+	va_end(ap);
 
-		rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s",
-			func_name, buffer);
-	}
+	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s", func_name, buffer);
 }
 
 /*