On 2/21/24 03:37, Pavel Cahyna wrote:
[ please Cc: me in replies, I am not subscribed and my attempt to
subscribe hits "Bug in Mailman version 2.1.29" .]

Hello Al,

certain fields are only used for static vs dynamic addresses, so only specific 
ones need to be initialized as long as things are programmed correctly.

IIRC, the patch below fixed the mistake that I used "source" instead of 
"source_type" for dynamic addresses.
that's my point - the patch below initializes "source_type" and leaves
"source" uninitialized, but then it keeps using "ipv6_data.source". To
me, the patch seems incomplete and
                                                   
get_dynamic_address_source_type_string (ipv6_data.source)) < 0)
looks like it should be changed to
                                                   
get_dynamic_address_source_type_string (ipv6_data.source_type)) < 0)

Ohhh, I get it now.  I misread the prior e-mail.  Yeah, I think you're right, that doesn't look right at all.

Thanks for the catch.  I can get this fixed sometime today.

I assume you hit this bug?  Is it critical for a new release?

Al

What do you think? If not, I don't see where does ipv6_data.source get
initialized.

Regards, Pavel

On Tue, Feb 20, 2024 at 05:03:46PM +0100, Pavel Cahyna wrote:
Hello,

I have a question about this commit:

commit 478ffaa7d2ffe240a0afaf9d7d5189342c94bd4d
Author: Albert Chu <ch...@llnl.gov>
Date:   Wed Jan 26 21:48:47 2022 -0800

     ipmi-config: correct IPv6 dynamic address checkout error


It contains this change:

-----
@@ -1089,15 +1090,15 @@ _get_ipv6_dynamic_address (ipmi_config_state_data_t 
*state_data,
        goto cleanup;
      }

-  if (FIID_OBJ_GET (obj_cmd_rs, "source", &val) < 0)
+  if (FIID_OBJ_GET (obj_cmd_rs, "source_type", &val) < 0)
      {
        pstdout_fprintf (state_data->pstate,
                         stderr,
-                       "fiid_obj_get: 'source': %s\n",
+                       "fiid_obj_get: 'source_type': %s\n",
                         fiid_obj_errormsg (obj_cmd_rs));
        goto cleanup;
      }
-  ipv6_data->source = val;
+  ipv6_data->source_type = val;

    if (fiid_obj_get_data (obj_cmd_rs,
                           "address",
-----


which, IIUC, leaves ipv6_data->source uninitialized.

It then contains this change:
-----
@@ -1186,7 +1187,7 @@ ipv6_dynamic_address_source_checkout 
(ipmi_config_state_data_t *state_data,

    if (ipmi_config_section_update_keyvalue_output (state_data,
                                                    kv,
-                                                  
get_dynamic_address_source_string (ipv6_data.source)) < 0)
+                                                  
get_dynamic_address_source_type_string (ipv6_data.source)) < 0)
      return (IPMI_CONFIG_ERR_FATAL_ERROR);

    rv = IPMI_CONFIG_ERR_SUCCESS;
-----

which keeps using ipv6_data.source, which looks uninitialized. This looks 
suspect, but I admit I
don't really understand the code.

Best regards, Pavel

--
Al Chu
Livermore Computing
Lawrence Livermore National Laboratory


Reply via email to