Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

librlist: workaround xml buffer size issue in some hwloc versions #5690

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Jan 18, 2024

Problem: With some versions of libxml, the XML load function does not tolerate a buffer length that includes the trailing NUL character. In hwloc 2.10.0, hwloc_topology_set_xmlbuffer() was changed to decrement the length before passing to the underlying XML call, so this effectively means that hwloc now requires the NUL character be included in length, but current librlist code does not include it to workaround the problem in v2.10.

Since there is no way to determine if NUL should or should not be included in hwloc_topology_set_xmlbuffer(), include the NUL by default, but retry without it if the first call fails.

Fixes #5689

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Wow, that's pretty poor API management.

Problem: With some versions of libxml, the XML load function does not
tolerate a buffer length that includes the trailing NUL character.
In hwloc 2.10.0, hwloc_topology_set_xmlbuffer() was changed to
decrement the length before passing to the underlying XML call, so
this effectively means that hwloc now *requires* the NUL character
be included in length, but current librlist code does not include it
to workaround the problem in v2.10.

Since there is no way to determine if NUL should or should not be
included in hwloc_topology_set_xmlbuffer(), include the NUL by default,
but retry without it if the first call fails.

Fixes flux-framework#5689
@grondo
Copy link
Contributor Author

grondo commented Jan 19, 2024

Thanks! Setting MWP.

Copy link

codecov bot commented Jan 19, 2024

Codecov Report

Merging #5690 (21b0fba) into master (f1feae8) will increase coverage by 0.00%.
The diff coverage is 40.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5690   +/-   ##
=======================================
  Coverage   83.46%   83.47%           
=======================================
  Files         485      485           
  Lines       82885    82890    +5     
=======================================
+ Hits        69184    69192    +8     
+ Misses      13701    13698    -3     
Files Coverage Δ
src/common/librlist/rhwloc.c 87.23% <40.00%> (-2.48%) ⬇️

... and 9 files with indirect coverage changes

@mergify mergify bot merged commit c4c427c into flux-framework:master Jan 19, 2024
32 of 33 checks passed
@grondo grondo deleted the issue#5689 branch January 19, 2024 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

librlist: test_rhwloc.t fails with hwloc 2.10
2 participants