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

Encoded show_help text causing compiler warnings #13167

Closed
jsquyres opened this issue Mar 25, 2025 · 6 comments · Fixed by #13178
Closed

Encoded show_help text causing compiler warnings #13167

jsquyres opened this issue Mar 25, 2025 · 6 comments · Fixed by #13178
Assignees
Milestone

Comments

@jsquyres
Copy link
Member

With older GCC (e.g., 8), we get warnings when compiling the new very-long strings from #13144:

 "The user is responsible for free'ing the returned string session ID." },
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
show_help_content.c:2723:1: warning: unknown escape sequence: '\;'
 "delimiters - e.g., \"my\;weird.nspace:5\"." },
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
show_help_content.c:3948:1: warning: string length '4136' is greater than the length '4095' ISO C99 compilers are required to support [-Woverlength-strings]
 "Provide all output in XML format" },
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
show_help_content.c:4964:1: warning: string length '5598' is greater than the length '4095' ISO C99 compilers are required to support [-Woverlength-strings]
 "CR_Pack_Nodes configuration parameter." },
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
show_help_content.c:5061:1: warning: string length '5488' is greater than the length '4095' ISO C99 compilers are required to support [-Woverlength-strings]
 "Show help message for cpu_bind" },
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
show_help_content.c:5312:13: warning: no previous prototype for 'opal_show_help_get_content' [-Wmissing-prototypes]
 const char *opal_show_help_get_content(const char *filename, const char* topic)
             ^~~~~~~~~~~~~~~~~~~~~~~~~~
@jsquyres jsquyres self-assigned this Mar 25, 2025
@jsquyres jsquyres added the bug label Mar 25, 2025
@jsquyres jsquyres added this to the v6.0.0 milestone Mar 25, 2025
@rhc54
Copy link
Contributor

rhc54 commented Mar 25, 2025

Do you know when those limits were raised, and to what size? The tools are going to soar past that limit for their "usage" topic.

@jsquyres
Copy link
Member Author

No. I was just going to split up the strings to be argv-style lists, each of which are <4095 bytes. The rendered results will be the same, but we'll be able to handle older compilers.

@rhc54
Copy link
Contributor

rhc54 commented Mar 31, 2025

FWIW: I fixed this in my version of your script - you can see it here: openpmix/openpmix#3577

It was a pretty simple fix. I also printed out the array in a more human-readable fashion for my convenience.

@rhc54
Copy link
Contributor

rhc54 commented Mar 31, 2025

Also note that I've had a lot of trouble getting thru the old CI based on Josh's docker images due to Python conflicts. Those old images do have Python 3 in them, but apparently before they supported all those cute formatting directives. So I've had to remove a bunch of them, replacing them with string concatenations. Might be a warning about compatibility with older Python 3 releases.

@jsquyres
Copy link
Member Author

jsquyres commented Apr 6, 2025

@rhc54 Thanks for the pointers!

If you care, I filed #13178 with the fixes here. It actually ended up simplifying things.

@rhc54
Copy link
Contributor

rhc54 commented Apr 6, 2025

I'll take a look at it and see what I can steal. My show-help is more complicated because of the hierarchical support (i.e., you can ask "--help foo" to get help on an option). I also added a bunch of checking in it to ensure we were using all the topics, weren't missing topics, the executables have all their options in their "--help" output, etc.

Figured if I'm going to spend a month pulling it all into memory, I might as well make sure it is all correct too! This way, it becomes a CI tool to double-check someone committing a new option or a new tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants