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

testprog.cpp: test case and fix for segfaults when using OptionGroup #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DrMicrobit
Copy link

Class OptionParser stored only pointers to option groups when using
add_option_group(). This can lead to the case where arguments are parsed
long after an OptionGroup went out of scope, leading to memory access to
freed memory and often to segmentation faults (the changed testprog.cpp
demonstrates this).

This can easily happen if a parser is configured in an external function
like this
auto parser = optparse::OptionParser();
setupParser(parser);

Fix:
Class OptionParser now stores option groups instead of pointers to them.

Pro:

  • least amount of changes to the code

Con:

  • Option groups cannot be changed after they were added to the parser
    via add_option_group(). Should this be needed, then a bigger rewrite
    would be necessary where the parser uses a factory function which
    creates and stores new option groups and returns a pointer/reference to
    them.

Class OptionParser stored only pointers to option groups when using
add_option_group(). This can lead to the case where arguments are parsed
long after an OptionGroup went out of scope, leading to memory access to
freed memory and often to segmentation faults (the changed testprog.cpp
demonstrates this).

This can easily happen if a parser is configured in an external function
like this
auto parser = optparse::OptionParser();
setupParser(parser);

Fix:
Class OptionParser now stores option groups instead of pointers to them.

Pro:
- least amount of changes to the code
Con:
- Option groups cannot be changed after they were added to the parser
via add_option_group(). Should this be needed, then a bigger rewrite
would be necessary where the parser uses a factory function which
creates and stores new option groups and returns a pointer/reference to
them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants