Hey Pavel,
That's awesome. Thanks!
Al
On 2/21/24 13:09, Pavel Cahyna wrote:
On Wed, Feb 21, 2024 at 09:50:35AM -0800, Al Chu wrote:
Yeah, to verify you'd need a BMC that has the ability to configure a dynamic
IP address.
I found that I have such a system, actually, so I tried the old and new
(with yout patch below) build.
Old:
# ipmi-config --checkout -e Lan6_Conf:IPv6_Dynamic_Address_Source_Type
Section Lan6_Conf
## READ-ONLY
## IPv6_Dynamic_Address_Source_Type
EndSection
# valgrind ipmi-config --checkout -e Lan6_Conf:IPv6_Dynamic_Address_Source_Type
==26033== ERROR SUMMARY: 264 errors from 46 contexts (suppressed: 0 from 0)
New:
# ipmi-config --checkout -e Lan6_Conf:IPv6_Dynamic_Address_Source_Type
Section Lan6_Conf
## READ-ONLY
## IPv6_Dynamic_Address_Source_Type SLAAC
EndSection
# valgrind ipmi-config --checkout -e Lan6_Conf:IPv6_Dynamic_Address_Source_Type
==25466== ERROR SUMMARY: 256 errors from 38 contexts (suppressed: 0 from 0)
Certainly looks like an improvement!
Thank you, Pavel
here's the patch i just commited upstream
https://urldefense.us/v3/__https://github.com/chu11/freeipmi-mirror/commit/41d0d70f09b4becfceef0517543cbf335c0e927a__;!!G2kpM7uM-TzIFchu!2HMNiudAcfaP44Pib4TZCpqSnrhLps0kj1vNwcboaXeZSNMQ9A_r50dIclZ2ssPfkCAuzKGS9UAX--Gj$
Thanks for the catch
Oh and I got an e-mail that said you subscribed to freeipmi-devel today?
I'm not sure what was up with gnu's mailman, but hopefully it's fixed now.
Al
On 2/21/24 09:25, Pavel Cahyna wrote:
On Wed, Feb 21, 2024 at 09:04:13AM -0800, Al Chu wrote:
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?
I have not hit the bug, I am not using ipmi-config myself, actually. The
bug was found by a static code scanner. Do you know how would one
reproduce it or in general test this code? I suppose this needs a BMC
with an IPv6 address configured?
It is not entirely critical, but I would prefer not to update to a
release which has a coding error added. If you have a patch, I will add
it to the CentOS Stream and Fedora packages.
Regards, Pavel
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
--
Al Chu
Livermore Computing
Lawrence Livermore National Laboratory
--
Al Chu
Livermore Computing
Lawrence Livermore National Laboratory
