diff mbox series

eal: fix warnings on Windows

Message ID 20200513225341.7620-1-pallavi.kadam@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal: fix warnings on Windows |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues

Commit Message

Kadam, Pallavi May 13, 2020, 10:53 p.m. UTC 
  This patch fixes bunch of warnings when compiling on Windows
such as the use of an unsafe string function (strerror),
[-Wunused-const-variable] in getopt.c and
[-Wunused-variable], [-Wunused-function] in eal_common_options.c

Signed-off-by: Ranjit Menon <ranjit.menon@intel.com>
Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 6 +++++-
 lib/librte_eal/windows/getopt.c            | 4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

Comments

Dmitry Kozlyuk May 14, 2020, 12:43 a.m. UTC | #1 
On Wed, 13 May 2020 15:53:41 -0700 Pallavi Kadam wrote:
> This patch fixes bunch of warnings when compiling on Windows
> such as the use of an unsafe string function (strerror),
> [-Wunused-const-variable] in getopt.c and
> [-Wunused-variable], [-Wunused-function] in eal_common_options.c
> 
> Signed-off-by: Ranjit Menon <ranjit.menon@intel.com>
> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>
> ---
>  lib/librte_eal/common/eal_common_options.c | 6 +++++-
>  lib/librte_eal/windows/getopt.c            | 4 ++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c index
> 8f2cbd1c6..2efbf59e4 100644 ---
> a/lib/librte_eal/common/eal_common_options.c +++
> b/lib/librte_eal/common/eal_common_options.c @@ -115,8 +115,10 @@
> struct shared_driver { static struct shared_driver_list solib_list =
>  TAILQ_HEAD_INITIALIZER(solib_list);
>  
> +#ifndef RTE_EXEC_ENV_WINDOWS
>  /* Default path of external loadable drivers */
>  static const char *default_solib_dir = RTE_EAL_PMD_PATH;
> +#endif
>  
>  /*
>   * Stringified version of solib path used by dpdk-pmdinfo.py
> @@ -329,6 +331,7 @@ eal_plugin_add(const char *path)
>  	return 0;
>  }
>  
> +#ifndef RTE_EXEC_ENV_WINDOWS
>  static int
>  eal_plugindir_init(const char *path)
>  {
> @@ -362,6 +365,7 @@ eal_plugindir_init(const char *path)
>  	/* XXX this ignores failures from readdir() itself */
>  	return (dent == NULL) ? 0 : -1;
>  }
> +#endif

This causes different warnings:

In file included from ../../../lib/librte_eal/common/eal_common_options.c:21:
At top level:
../../../lib/librte_eal/windows/include/dirent.h:529:1: warning: 'closedir' defined but not used [-Wunused-function]
  529 | closedir(DIR *dirp)
      | ^~~~~~~~
../../../lib/librte_eal/windows/include/dirent.h:447:1: warning: 'readdir' defined but not used [-Wunused-function]
  447 | readdir(DIR *dirp)
      | ^~~~~~~
../../../lib/librte_eal/windows/include/dirent.h:377:1: warning: 'opendir' defined but not used [-Wunused-function]
  377 | opendir(const char *dirname)
      | ^~~~~~~

Suggesting not to include <dirent.h> to this file on Windows (this
makes dirent.h unused in EAL, but other code will require it later).

>  
>  int
>  eal_plugins_init(void)
> @@ -394,8 +398,8 @@ eal_plugins_init(void)
>  		}
>  
>  	}
> -	return 0;
>  #endif
> +	return 0;
>  }
>  
>  /*
> diff --git a/lib/librte_eal/windows/getopt.c
> b/lib/librte_eal/windows/getopt.c index 170c9b5e0..a08f7c109 100644
> --- a/lib/librte_eal/windows/getopt.c
> +++ b/lib/librte_eal/windows/getopt.c
> @@ -25,8 +25,8 @@ int	opterr = 1;		/* if error
> message should be printed */ int	optind = 1;		/*
> index into parent argv vector */ int	optopt = '?';
> 	/* character checked for validity */ 
> -static void pass(void) {}
> -#define warnx(a, ...) pass()
> +static void pass(const char *a) {(void) a; }
> +#define warnx(a, ...) pass(a)
>  
>  #define PRINT_ERROR	((opterr) && (*options != ':'))
>  

Testing with MinGW-w64 (Ubuntu 19.10, package 6.0.0-3, GCC 9.2.1)
reveals additional warnings. Never mind fixing them in a separate patch
to avoid blocking this one. For the record:

../../../lib/librte_eal/common/eal_common_thread.c:180:25: warning: unused parameter 'attr' [-Wunused-parameter]

../../../lib/librte_eal/windows/eal_lcore.c:30:1: warning: old-style function definition [-Wold-style-definition]
   30 | eal_create_cpu_map()

../../../lib/librte_eal/windows/eal_thread.c:149:29: warning: cast between incompatible function types from '__attribute__((noreturn)) void * (*)(void *)' to 'DWORD (*)(void *)' {aka 'long unsigned int (*)(void * '} [-Wcast-function-type]
  149 |  th = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)eal_thread_loop,

../../../lib/librte_eal/windows/eal.c:144:1: warning: 'optnone' attribute directive ignored [-Wattributes]

../../../lib/librte_eal/common/eal_common_options.c: In function 'eal_adjust_config':
../../../lib/librte_eal/windows/include/sched.h:63:55: warning: 'default_set._bits[1]' may be used uninitialized in this function [-Wmaybe-uninitialized]
   63 |   (dst)->_bits[_i] = (src1)->_bits[_i] & (src2)->_bits[_i]; \

--
Dmitry Kozlyuk
Menon, Ranjit May 14, 2020, 1:24 a.m. UTC | #2 
On 5/13/2020 5:43 PM, Dmitry Kozlyuk wrote:
> On Wed, 13 May 2020 15:53:41 -0700 Pallavi Kadam wrote:
>> This patch fixes bunch of warnings when compiling on Windows
>> such as the use of an unsafe string function (strerror),
>> [-Wunused-const-variable] in getopt.c and
>> [-Wunused-variable], [-Wunused-function] in eal_common_options.c
>>
>> Signed-off-by: Ranjit Menon <ranjit.menon@intel.com>
>> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
>> Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>
>> ---
>>   lib/librte_eal/common/eal_common_options.c | 6 +++++-
>>   lib/librte_eal/windows/getopt.c            | 4 ++--
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_options.c
>> b/lib/librte_eal/common/eal_common_options.c index
>> 8f2cbd1c6..2efbf59e4 100644 ---
>> a/lib/librte_eal/common/eal_common_options.c +++
>> b/lib/librte_eal/common/eal_common_options.c @@ -115,8 +115,10 @@
>> struct shared_driver { static struct shared_driver_list solib_list =
>>   TAILQ_HEAD_INITIALIZER(solib_list);
>>   
>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>   /* Default path of external loadable drivers */
>>   static const char *default_solib_dir = RTE_EAL_PMD_PATH;
>> +#endif
>>   
>>   /*
>>    * Stringified version of solib path used by dpdk-pmdinfo.py
>> @@ -329,6 +331,7 @@ eal_plugin_add(const char *path)
>>   	return 0;
>>   }
>>   
>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>   static int
>>   eal_plugindir_init(const char *path)
>>   {
>> @@ -362,6 +365,7 @@ eal_plugindir_init(const char *path)
>>   	/* XXX this ignores failures from readdir() itself */
>>   	return (dent == NULL) ? 0 : -1;
>>   }
>> +#endif
> 
> This causes different warnings:
> 
> In file included from ../../../lib/librte_eal/common/eal_common_options.c:21:
> At top level:
> ../../../lib/librte_eal/windows/include/dirent.h:529:1: warning: 'closedir' defined but not used [-Wunused-function]
>    529 | closedir(DIR *dirp)
>        | ^~~~~~~~
> ../../../lib/librte_eal/windows/include/dirent.h:447:1: warning: 'readdir' defined but not used [-Wunused-function]
>    447 | readdir(DIR *dirp)
>        | ^~~~~~~
> ../../../lib/librte_eal/windows/include/dirent.h:377:1: warning: 'opendir' defined but not used [-Wunused-function]
>    377 | opendir(const char *dirname)
>        | ^~~~~~~
> 
> Suggesting not to include <dirent.h> to this file on Windows (this
> makes dirent.h unused in EAL, but other code will require it later).
> 

Yes. We saw this too and realized that if we put a #ifndef 
RTE_EXEC_ENV_WINDOWS around the #include of dirent.h, it would render 
the file as being unused. But, I think it makes sense to do it - better 
than having these warnings.

>>   
>>   int
>>   eal_plugins_init(void)
>> @@ -394,8 +398,8 @@ eal_plugins_init(void)
>>   		}
>>   
>>   	}
>> -	return 0;
>>   #endif
>> +	return 0;
>>   }
>>   
>>   /*
>> diff --git a/lib/librte_eal/windows/getopt.c
>> b/lib/librte_eal/windows/getopt.c index 170c9b5e0..a08f7c109 100644
>> --- a/lib/librte_eal/windows/getopt.c
>> +++ b/lib/librte_eal/windows/getopt.c
>> @@ -25,8 +25,8 @@ int	opterr = 1;		/* if error
>> message should be printed */ int	optind = 1;		/*
>> index into parent argv vector */ int	optopt = '?';
>> 	/* character checked for validity */
>> -static void pass(void) {}
>> -#define warnx(a, ...) pass()
>> +static void pass(const char *a) {(void) a; }
>> +#define warnx(a, ...) pass(a)
>>   
>>   #define PRINT_ERROR	((opterr) && (*options != ':'))
>>   
> 
> Testing with MinGW-w64 (Ubuntu 19.10, package 6.0.0-3, GCC 9.2.1)
> reveals additional warnings. Never mind fixing them in a separate patch
> to avoid blocking this one. For the record:
> 
> ../../../lib/librte_eal/common/eal_common_thread.c:180:25: warning: unused parameter 'attr' [-Wunused-parameter]
> 
> ../../../lib/librte_eal/windows/eal_lcore.c:30:1: warning: old-style function definition [-Wold-style-definition]
>     30 | eal_create_cpu_map()
> 
> ../../../lib/librte_eal/windows/eal_thread.c:149:29: warning: cast between incompatible function types from '__attribute__((noreturn)) void * (*)(void *)' to 'DWORD (*)(void *)' {aka 'long unsigned int (*)(void * '} [-Wcast-function-type]
>    149 |  th = CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)eal_thread_loop,
> 
> ../../../lib/librte_eal/windows/eal.c:144:1: warning: 'optnone' attribute directive ignored [-Wattributes]
> 
> ../../../lib/librte_eal/common/eal_common_options.c: In function 'eal_adjust_config':
> ../../../lib/librte_eal/windows/include/sched.h:63:55: warning: 'default_set._bits[1]' may be used uninitialized in this function [-Wmaybe-uninitialized]
>     63 |   (dst)->_bits[_i] = (src1)->_bits[_i] & (src2)->_bits[_i]; \
> 
> --
> Dmitry Kozlyuk
> 

Yes, we can address this in a separate patch.

ranjit m.
Thomas Monjalon May 14, 2020, 8:02 a.m. UTC | #3 
14/05/2020 00:53, Pallavi Kadam:
> This patch fixes bunch of warnings when compiling on Windows
> such as the use of an unsafe string function (strerror),
> [-Wunused-const-variable] in getopt.c and
> [-Wunused-variable], [-Wunused-function] in eal_common_options.c
> 
> Signed-off-by: Ranjit Menon <ranjit.menon@intel.com>
> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>
> ---
> +#ifndef RTE_EXEC_ENV_WINDOWS
>  static int
>  eal_plugindir_init(const char *path)

Why disabling the plugin mechanism?
Can we make it working instead?

PS: When sending patches for Windows, please Cc
the official Windows maintainers and Dmitry Kozliuk.
Menon, Ranjit May 14, 2020, 5:46 p.m. UTC | #4 
On 5/14/2020 1:02 AM, Thomas Monjalon wrote:
> 14/05/2020 00:53, Pallavi Kadam:
>> This patch fixes bunch of warnings when compiling on Windows
>> such as the use of an unsafe string function (strerror),
>> [-Wunused-const-variable] in getopt.c and
>> [-Wunused-variable], [-Wunused-function] in eal_common_options.c
>>
>> Signed-off-by: Ranjit Menon <ranjit.menon@intel.com>
>> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
>> Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>
>> ---
>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>   static int
>>   eal_plugindir_init(const char *path)
> 
> Why disabling the plugin mechanism?
> Can we make it working instead?
> 

The function that is calling eal_plugindir_init() has already been 
#ifndef-ed out for Windows compilation. So, this function will never be 
called on Windows and was the cause of multiple compilation warnings.
Best to keep it disabled until we can have this implemented and working 
on Windows

> PS: When sending patches for Windows, please Cc
> the official Windows maintainers and Dmitry Kozliuk.
> 
> 

Yes. Will do.

ranjit m.
diff mbox series

Patch

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 8f2cbd1c6..2efbf59e4 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -115,8 +115,10 @@  struct shared_driver {
 static struct shared_driver_list solib_list =
 TAILQ_HEAD_INITIALIZER(solib_list);
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 /* Default path of external loadable drivers */
 static const char *default_solib_dir = RTE_EAL_PMD_PATH;
+#endif
 
 /*
  * Stringified version of solib path used by dpdk-pmdinfo.py
@@ -329,6 +331,7 @@  eal_plugin_add(const char *path)
 	return 0;
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eal_plugindir_init(const char *path)
 {
@@ -362,6 +365,7 @@  eal_plugindir_init(const char *path)
 	/* XXX this ignores failures from readdir() itself */
 	return (dent == NULL) ? 0 : -1;
 }
+#endif
 
 int
 eal_plugins_init(void)
@@ -394,8 +398,8 @@  eal_plugins_init(void)
 		}
 
 	}
-	return 0;
 #endif
+	return 0;
 }
 
 /*
diff --git a/lib/librte_eal/windows/getopt.c b/lib/librte_eal/windows/getopt.c
index 170c9b5e0..a08f7c109 100644
--- a/lib/librte_eal/windows/getopt.c
+++ b/lib/librte_eal/windows/getopt.c
@@ -25,8 +25,8 @@  int	opterr = 1;		/* if error message should be printed */
 int	optind = 1;		/* index into parent argv vector */
 int	optopt = '?';		/* character checked for validity */
 
-static void pass(void) {}
-#define warnx(a, ...) pass()
+static void pass(const char *a) {(void) a; }
+#define warnx(a, ...) pass(a)
 
 #define PRINT_ERROR	((opterr) && (*options != ':'))