Skip to content

Option segfault and usage cleanup#34

Closed
tsibley wants to merge 4 commits intonajoshi:masterfrom
MullinsLab:option-segfault-and-usage-cleanup
Closed

Option segfault and usage cleanup#34
tsibley wants to merge 4 commits intonajoshi:masterfrom
MullinsLab:option-segfault-and-usage-cleanup

Conversation

@tsibley
Copy link
Copy Markdown

@tsibley tsibley commented Aug 15, 2014

See commit messages for description.

Many options which required an argument were declared as taking an
optional argument.  The option handling loop, however, did not expect
optional arguments.  For example, passing "--qual-threshold 30" rather
than "--qual-threshold=30" or "-q 30" led to a segfault when atoi() was
passed NULL (optarg).  There were other affected options as well.

This commit correctly declares the required/optional/none status of
option arguments and matches that short-form option spec.

Presumably this was confusion between the entire option being optional
and the option's argument being optional.  The required option checking
is done outside of getopt.
This will help keep them in sync when updating options.
The usage docs now indicate option arguments and are easier to read.  If
the usage was specifically requested with --help, then it is printed to
stdout instead of stderr.  This is useful for the common idiom of asking
for help and piping to a pager like less or more (without redirecting
stderr).
Silences warnings about //-style comments and long strings.

Since kseq.h uses inline functions, a feature of C99, it's not useful
pretending to be C89 compat (GCC's default).
@tsibley tsibley closed this Feb 25, 2015
@tsibley tsibley deleted the option-segfault-and-usage-cleanup branch February 25, 2015 19:00
@tsibley tsibley restored the option-segfault-and-usage-cleanup branch February 25, 2015 19:00
@tsibley tsibley deleted the option-segfault-and-usage-cleanup branch February 25, 2015 19:00
@tsibley
Copy link
Copy Markdown
Author

tsibley commented Feb 25, 2015

Closing this in favor of #40, given the new commit from @najoshi: 972a0e3

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.

1 participant