diff mbox series

[iwl-net] ice: health.c: fix compilation on gcc 7.5

Message ID 20250205104252.30464-2-przemyslaw.kitszel@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [iwl-net] ice: health.c: fix compilation on gcc 7.5 | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 1 blamed authors not CCed: marcin.szycik@linux.intel.com; 8 maintainers not CCed: igor.bagnucki@intel.com mateusz.polchlopek@intel.com andrew+netdev@lunn.ch edumazet@google.com wojciech.drewek@intel.com kuba@kernel.org marcin.szycik@linux.intel.com pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 2 this patch: 2
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Przemek Kitszel Feb. 5, 2025, 10:42 a.m. UTC
GCC 7 is not as good as GCC 8+ in telling what is a compile-time const,
and thus could be used for static storage. So we could not use variables
for that, no matter how much "const" keyword is sprinkled around.

Excerpt from the report:
My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.

  CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element is not constant
   ice_common_port_solutions, {ice_port_number_label}},
   ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization for 'ice_health_status_lookup[0].solution')
drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer element is not constant
   ice_common_port_solutions, {ice_port_number_label}},
                               ^~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization for 'ice_health_status_lookup[0].data_label[0]')
drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer element is not constant
   "Change or replace the module or cable.", {ice_port_number_label}},
                                              ^~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization for 'ice_health_status_lookup[1].data_label[0]')
drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element is not constant
   ice_common_port_solutions, {ice_port_number_label}},
   ^~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Closes: https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F72@CY8PR11MB7134.namprd11.prod.outlook.com
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
I would really like to bump min gcc to 8.5 (RH 8 family),
instead of supporting old Ubuntu. However SLES 15 is also stuck with gcc 7.5 :(

CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Kees Cook <kees@kernel.org>
CC: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 drivers/net/ethernet/intel/ice/devlink/health.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


base-commit: 4241a702e0d0c2ca9364cfac08dbf134264962de

Comments

Michal Swiatkowski Feb. 5, 2025, 11:19 a.m. UTC | #1
On Wed, Feb 05, 2025 at 11:42:12AM +0100, Przemek Kitszel wrote:
> GCC 7 is not as good as GCC 8+ in telling what is a compile-time const,
> and thus could be used for static storage. So we could not use variables
> for that, no matter how much "const" keyword is sprinkled around.
> 
> Excerpt from the report:
> My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.
> 
>   CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization for 'ice_health_status_lookup[0].solution')
> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>                                ^~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization for 'ice_health_status_lookup[0].data_label[0]')
> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer element is not constant
>    "Change or replace the module or cable.", {ice_port_number_label}},
>                                               ^~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization for 'ice_health_status_lookup[1].data_label[0]')
> drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
> Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> Closes: https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F72@CY8PR11MB7134.namprd11.prod.outlook.com
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> I would really like to bump min gcc to 8.5 (RH 8 family),
> instead of supporting old Ubuntu. However SLES 15 is also stuck with gcc 7.5 :(
> 
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Kees Cook <kees@kernel.org>
> CC: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  drivers/net/ethernet/intel/ice/devlink/health.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
> index ea40f7941259..4bc546bafad1 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c
> @@ -23,12 +23,12 @@ struct ice_health_status {
>   * For instance, Health Code 0x1002 is triggered when the command fails.
>   * Such codes should be disregarded by the end-user.
>   * The below lookup requires to be sorted by code.
> + * #defines instead of proper const strings are used due to gcc 7 limitation.
>   */
>  
> -static const char *const ice_common_port_solutions =
> -	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex.";
> -static const char *const ice_port_number_label = "Port Number";
> -static const char *const ice_update_nvm_solution = "Update to the latest NVM image.";
> +#define ice_common_port_solutions	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex."
> +#define ice_port_number_label		"Port Number"
> +#define ice_update_nvm_solution		"Update to the latest NVM image."

>  
>  static const struct ice_health_status ice_health_status_lookup[] = {
>  	{ICE_AQC_HEALTH_STATUS_ERR_UNKNOWN_MOD_STRICT, "An unsupported module was detected.",
> 
> base-commit: 4241a702e0d0c2ca9364cfac08dbf134264962de

Thanks for fixing
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> -- 
> 2.46.0
Zhuo, Qiuxu Feb. 5, 2025, 11:39 a.m. UTC | #2
> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> [...]
> Subject: [PATCH iwl-net] ice: health.c: fix compilation on gcc 7.5
> 
> GCC 7 is not as good as GCC 8+ in telling what is a compile-time const, and
> thus could be used for static storage. So we could not use variables for that,
> no matter how much "const" keyword is sprinkled around.
> 
> Excerpt from the report:
> My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.
> 
>   CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element
> is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization
> for 'ice_health_status_lookup[0].solution')
> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer
> element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>                                ^~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization
> for 'ice_health_status_lookup[0].data_label[0]')
> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer
> element is not constant
>    "Change or replace the module or cable.", {ice_port_number_label}},
>                                               ^~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization
> for 'ice_health_status_lookup[1].data_label[0]')
> drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element
> is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
> Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> Closes:
> https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F
> 72@CY8PR11MB7134.namprd11.prod.outlook.com
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Tested-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>

[...]
Simon Horman Feb. 5, 2025, 8:45 p.m. UTC | #3
+ Jiri

On Wed, Feb 05, 2025 at 11:42:12AM +0100, Przemek Kitszel wrote:
> GCC 7 is not as good as GCC 8+ in telling what is a compile-time const,
> and thus could be used for static storage. So we could not use variables
> for that, no matter how much "const" keyword is sprinkled around.
> 
> Excerpt from the report:
> My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.
> 
>   CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization for 'ice_health_status_lookup[0].solution')
> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>                                ^~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization for 'ice_health_status_lookup[0].data_label[0]')
> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer element is not constant
>    "Change or replace the module or cable.", {ice_port_number_label}},
>                                               ^~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization for 'ice_health_status_lookup[1].data_label[0]')
> drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element is not constant
>    ice_common_port_solutions, {ice_port_number_label}},
>    ^~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
> Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> Closes: https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F72@CY8PR11MB7134.namprd11.prod.outlook.com
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
> I would really like to bump min gcc to 8.5 (RH 8 family),
> instead of supporting old Ubuntu. However SLES 15 is also stuck with gcc 7.5 :(
> 
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Kees Cook <kees@kernel.org>
> CC: Nick Desaulniers <nick.desaulniers@gmail.com>

Hi Prezemek,

I ran into a similar problem not so long ago and I'm wondering if
the following, based on a suggestion by Jiri Slaby, resolves your
problem.

diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
index ea40f7941259..19c3d37aa768 100644
--- a/drivers/net/ethernet/intel/ice/devlink/health.c
+++ b/drivers/net/ethernet/intel/ice/devlink/health.c
@@ -25,10 +25,10 @@ struct ice_health_status {
  * The below lookup requires to be sorted by code.
  */
 
-static const char *const ice_common_port_solutions =
+static const char ice_common_port_solutions[] =
 	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex.";
-static const char *const ice_port_number_label = "Port Number";
-static const char *const ice_update_nvm_solution = "Update to the latest NVM image.";
+static const char ice_port_number_label[] = "Port Number";
+static const char ice_update_nvm_solution[] = "Update to the latest NVM image.";
 
 static const struct ice_health_status ice_health_status_lookup[] = {
 	{ICE_AQC_HEALTH_STATUS_ERR_UNKNOWN_MOD_STRICT, "An unsupported module was detected.",


Link: https://lore.kernel.org/netdev/485dbc5a-a04b-40c2-9481-955eaa5ce2e2@kernel.org/
Link: https://git.kernel.org/netdev/net-next/c/36fb51479e3c
David Laight Feb. 5, 2025, 10:56 p.m. UTC | #4
On Wed, 5 Feb 2025 20:45:46 +0000
Simon Horman <horms@kernel.org> wrote:

> + Jiri
> 
> On Wed, Feb 05, 2025 at 11:42:12AM +0100, Przemek Kitszel wrote:
> > GCC 7 is not as good as GCC 8+ in telling what is a compile-time const,
> > and thus could be used for static storage. So we could not use variables
> > for that, no matter how much "const" keyword is sprinkled around.
> > 
> > Excerpt from the report:
> > My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.
> > 
> >   CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
> > drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element is not constant
> >    ice_common_port_solutions, {ice_port_number_label}},
> >    ^~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization for 'ice_health_status_lookup[0].solution')
> > drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer element is not constant
> >    ice_common_port_solutions, {ice_port_number_label}},
> >                                ^~~~~~~~~~~~~~~~~~~~~
> > drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization for 'ice_health_status_lookup[0].data_label[0]')
> > drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer element is not constant
> >    "Change or replace the module or cable.", {ice_port_number_label}},
> >                                               ^~~~~~~~~~~~~~~~~~~~~
> > drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization for 'ice_health_status_lookup[1].data_label[0]')
> > drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element is not constant
> >    ice_common_port_solutions, {ice_port_number_label}},
> >    ^~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
> > Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> > Closes: https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F72@CY8PR11MB7134.namprd11.prod.outlook.com
> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > ---
> > I would really like to bump min gcc to 8.5 (RH 8 family),
> > instead of supporting old Ubuntu. However SLES 15 is also stuck with gcc 7.5 :(
> > 
> > CC: Linus Torvalds <torvalds@linux-foundation.org>
> > CC: Kees Cook <kees@kernel.org>
> > CC: Nick Desaulniers <nick.desaulniers@gmail.com>  
> 
> Hi Prezemek,
> 
> I ran into a similar problem not so long ago and I'm wondering if
> the following, based on a suggestion by Jiri Slaby, resolves your
> problem.

I'm sure I remember from somewhere that although the variables are
'static const' they have to be real variables because they can still
be patched.

Which stops you using their contents as initialisers.

Maybe I'm mis-remembering it.

	David

> 
> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
> index ea40f7941259..19c3d37aa768 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c
> @@ -25,10 +25,10 @@ struct ice_health_status {
>   * The below lookup requires to be sorted by code.
>   */
>  
> -static const char *const ice_common_port_solutions =
> +static const char ice_common_port_solutions[] =
>  	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex.";
> -static const char *const ice_port_number_label = "Port Number";
> -static const char *const ice_update_nvm_solution = "Update to the latest NVM image.";
> +static const char ice_port_number_label[] = "Port Number";
> +static const char ice_update_nvm_solution[] = "Update to the latest NVM image.";
>  
>  static const struct ice_health_status ice_health_status_lookup[] = {
>  	{ICE_AQC_HEALTH_STATUS_ERR_UNKNOWN_MOD_STRICT, "An unsupported module was detected.",
> 
> 
> Link: https://lore.kernel.org/netdev/485dbc5a-a04b-40c2-9481-955eaa5ce2e2@kernel.org/
> Link: https://git.kernel.org/netdev/net-next/c/36fb51479e3c
>
Jiri Slaby Feb. 6, 2025, 7:45 a.m. UTC | #5
On 05. 02. 25, 21:45, Simon Horman wrote:
> + Jiri
> 
> On Wed, Feb 05, 2025 at 11:42:12AM +0100, Przemek Kitszel wrote:
>> GCC 7 is not as good as GCC 8+ in telling what is a compile-time const,
>> and thus could be used for static storage. So we could not use variables
>> for that, no matter how much "const" keyword is sprinkled around.
>>
>> Excerpt from the report:
>> My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.
>>
>>    CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
>> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element is not constant
>>     ice_common_port_solutions, {ice_port_number_label}},
>>     ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization for 'ice_health_status_lookup[0].solution')
>> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer element is not constant
>>     ice_common_port_solutions, {ice_port_number_label}},
>>                                 ^~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization for 'ice_health_status_lookup[0].data_label[0]')
>> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer element is not constant
>>     "Change or replace the module or cable.", {ice_port_number_label}},
>>                                                ^~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization for 'ice_health_status_lookup[1].data_label[0]')
>> drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element is not constant
>>     ice_common_port_solutions, {ice_port_number_label}},
>>     ^~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
>> Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
>> Closes: https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F72@CY8PR11MB7134.namprd11.prod.outlook.com
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>> I would really like to bump min gcc to 8.5 (RH 8 family),
>> instead of supporting old Ubuntu. However SLES 15 is also stuck with gcc 7.5 :(
>>
>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>> CC: Kees Cook <kees@kernel.org>
>> CC: Nick Desaulniers <nick.desaulniers@gmail.com>
> 
> Hi Prezemek,
> 
> I ran into a similar problem not so long ago and I'm wondering if
> the following, based on a suggestion by Jiri Slaby, resolves your
> problem.
> 
> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
> index ea40f7941259..19c3d37aa768 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c
> @@ -25,10 +25,10 @@ struct ice_health_status {
>    * The below lookup requires to be sorted by code.
>    */
>   
> -static const char *const ice_common_port_solutions =
> +static const char ice_common_port_solutions[] =
>   	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex.";
> -static const char *const ice_port_number_label = "Port Number";
> -static const char *const ice_update_nvm_solution = "Update to the latest NVM image.";
> +static const char ice_port_number_label[] = "Port Number";
> +static const char ice_update_nvm_solution[] = "Update to the latest NVM image.";

Indeed, no reason to create an (unused) pointer.

And no, don't use macros for strings.

thanks,
Jiri Slaby Feb. 6, 2025, 7:53 a.m. UTC | #6
On 05. 02. 25, 23:56, David Laight wrote:
> On Wed, 5 Feb 2025 20:45:46 +0000
> Simon Horman <horms@kernel.org> wrote:
> 
>> + Jiri
>>
>> On Wed, Feb 05, 2025 at 11:42:12AM +0100, Przemek Kitszel wrote:
>>> GCC 7 is not as good as GCC 8+ in telling what is a compile-time const,
>>> and thus could be used for static storage. So we could not use variables
>>> for that, no matter how much "const" keyword is sprinkled around.
>>>
>>> Excerpt from the report:
>>> My GCC is: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0.
>>>
>>>    CC [M]  drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.o
>>> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: error: initializer element is not constant
>>>     ice_common_port_solutions, {ice_port_number_label}},
>>>     ^~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/net/ethernet/intel/ice/devlink/health.c:35:3: note: (near initialization for 'ice_health_status_lookup[0].solution')
>>> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: error: initializer element is not constant
>>>     ice_common_port_solutions, {ice_port_number_label}},
>>>                                 ^~~~~~~~~~~~~~~~~~~~~
>>> drivers/net/ethernet/intel/ice/devlink/health.c:35:31: note: (near initialization for 'ice_health_status_lookup[0].data_label[0]')
>>> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: error: initializer element is not constant
>>>     "Change or replace the module or cable.", {ice_port_number_label}},
>>>                                                ^~~~~~~~~~~~~~~~~~~~~
>>> drivers/net/ethernet/intel/ice/devlink/health.c:37:46: note: (near initialization for 'ice_health_status_lookup[1].data_label[0]')
>>> drivers/net/ethernet/intel/ice/devlink/health.c:39:3: error: initializer element is not constant
>>>     ice_common_port_solutions, {ice_port_number_label}},
>>>     ^~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Fixes: 85d6164ec56d ("ice: add fw and port health reporters")
>>> Reported-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
>>> Closes: https://lore.kernel.org/netdev/CY8PR11MB7134BF7A46D71E50D25FA7A989F72@CY8PR11MB7134.namprd11.prod.outlook.com
>>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> ---
>>> I would really like to bump min gcc to 8.5 (RH 8 family),
>>> instead of supporting old Ubuntu. However SLES 15 is also stuck with gcc 7.5 :(
>>>
>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>> CC: Kees Cook <kees@kernel.org>
>>> CC: Nick Desaulniers <nick.desaulniers@gmail.com>
>>
>> Hi Prezemek,
>>
>> I ran into a similar problem not so long ago and I'm wondering if
>> the following, based on a suggestion by Jiri Slaby, resolves your
>> problem.
> 
> I'm sure I remember from somewhere that although the variables are
> 'static const' they have to be real variables because they can still
> be patched.

Not sure what you mean -- using macros, they placed the strings into 
.rodata anyway.

Mind the difference between:
const char *X;
char *const X;
const char *const X;

They are all different and allow different things (X++, (*X)++, nothing).

Possibly, the problem above could be fixed by the third variant too, 
IMO. But is ineffective (having generated an unused pointer).

thanks,
Kees Cook Feb. 6, 2025, 6:57 p.m. UTC | #7
On Wed, Feb 05, 2025 at 08:45:46PM +0000, Simon Horman wrote:
> I ran into a similar problem not so long ago and I'm wondering if
> the following, based on a suggestion by Jiri Slaby, resolves your
> problem.
> 
> diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
> index ea40f7941259..19c3d37aa768 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/health.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/health.c
> @@ -25,10 +25,10 @@ struct ice_health_status {
>   * The below lookup requires to be sorted by code.
>   */
>  
> -static const char *const ice_common_port_solutions =
> +static const char ice_common_port_solutions[] =
>  	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex.";
> -static const char *const ice_port_number_label = "Port Number";
> -static const char *const ice_update_nvm_solution = "Update to the latest NVM image.";
> +static const char ice_port_number_label[] = "Port Number";
> +static const char ice_update_nvm_solution[] = "Update to the latest NVM image.";
>  
>  static const struct ice_health_status ice_health_status_lookup[] = {
>  	{ICE_AQC_HEALTH_STATUS_ERR_UNKNOWN_MOD_STRICT, "An unsupported module was detected.",
> 

I'd agree that would be the preferred fix. :)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/devlink/health.c b/drivers/net/ethernet/intel/ice/devlink/health.c
index ea40f7941259..4bc546bafad1 100644
--- a/drivers/net/ethernet/intel/ice/devlink/health.c
+++ b/drivers/net/ethernet/intel/ice/devlink/health.c
@@ -23,12 +23,12 @@  struct ice_health_status {
  * For instance, Health Code 0x1002 is triggered when the command fails.
  * Such codes should be disregarded by the end-user.
  * The below lookup requires to be sorted by code.
+ * #defines instead of proper const strings are used due to gcc 7 limitation.
  */
 
-static const char *const ice_common_port_solutions =
-	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex.";
-static const char *const ice_port_number_label = "Port Number";
-static const char *const ice_update_nvm_solution = "Update to the latest NVM image.";
+#define ice_common_port_solutions	"Check your cable connection. Change or replace the module or cable. Manually set speed and duplex."
+#define ice_port_number_label		"Port Number"
+#define ice_update_nvm_solution		"Update to the latest NVM image."
 
 static const struct ice_health_status ice_health_status_lookup[] = {
 	{ICE_AQC_HEALTH_STATUS_ERR_UNKNOWN_MOD_STRICT, "An unsupported module was detected.",