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

convert to getopt_long for option parsing #871

Closed
namhyung opened this issue Sep 15, 2019 · 12 comments
Closed

convert to getopt_long for option parsing #871

namhyung opened this issue Sep 15, 2019 · 12 comments
Assignees
Milestone

Comments

@namhyung
Copy link
Owner

Currently it uses argp_parse() to parse command line options. But it's glibc-specific so has some portability problems. While there's a supporting library for this, it'd be nice to avoid using it. I think it's rather straight-forward to convert the option parser to use getopt_long() instead.

@honggyukim
Copy link
Collaborator

This is one of the most valuable work items as a first step to support Android.

@binkoni
Copy link
Contributor

binkoni commented Sep 17, 2019

I want to contribute this issue!

@honggyukim
Copy link
Collaborator

I want to contribute this issue!

Great! Please go ahead.

@namhyung
Copy link
Owner Author

@binkoni can I take this over?

@binkoni
Copy link
Contributor

binkoni commented Mar 29, 2020 via email

@namhyung namhyung assigned namhyung and unassigned binkoni Mar 29, 2020
namhyung added a commit that referenced this issue Mar 30, 2020
The argp_parse() has portability issues as some systems miss the
implementation by default.  So convert it to getopt_long() for better
compatibility.  It needs more manual work when you add a new option,
but it'd be more flexible to handle help messages at least.

The notable change is that it now requires subcommand name as the
first argument.  This is somewhat annoying for me but I don't know how
I can fix it with getopt().

So the following example will not work anymore (sadly).

  $ ./uftrace -L . record tests/t-abc

It should be like below:

  $ ./uftrace record -L . tests/t-abc

Closed: #871

Signed-off-by: Namhyung Kim <[email protected]>
@namhyung
Copy link
Owner Author

Pushed review/getopt-v1.

@honggyukim
Copy link
Collaborator

Thanks a lot for the update. It mostly works fine, but it doesn't properly pass target options.

$ uftrace record -P. ./chrome --no-sandbox --headless https://www.google.com
uftrace: unrecognized option '--no-sandbox'
uftrace: /home/honggyu/work/uftrace/cmds/record.c:1542:find_in_path
 ERROR: Cannot trace '--headless': No such executable file.

It requires explicit -- mark as follows.

$ uftrace record -P. -- ./chrome --no-sandbox --headless https://www.google.com

@honggyukim
Copy link
Collaborator

The simpler example is as follows.

$ gcc -pg -o t-abc tests/s-abc.c

$ ./t-abc
$ uftrace ./t-abc
# DURATION     TID     FUNCTION
   1.459 us [ 19883] | __monstartup();
   1.019 us [ 19883] | __cxa_atexit();
            [ 19883] | main() {
            [ 19883] |   a() {
            [ 19883] |     b() {
            [ 19883] |       c() {
   0.815 us [ 19883] |         getpid();
   2.005 us [ 19883] |       } /* c */
   2.578 us [ 19883] |     } /* b */
   3.002 us [ 19883] |   } /* a */
   3.473 us [ 19883] | } /* main */

The following --target-option is expected to be used as an option for t-abc itself instead of uftrace option, but it doesn't work.

$ ./t-abc --target-option
$ uftrace ./t-abc --target-option
uftrace: unrecognized option '--target-option'
uftrace -- function (graph) tracer for userspace

Usage: uftrace [COMMAND] [OPTION...] [<program>]

 COMMAND: record|replay|live|report|graph|info|dump|recv|script|tui

@namhyung
Copy link
Owner Author

Thanks for testing. I don't know how I can control that..

@namhyung
Copy link
Owner Author

namhyung commented Apr 6, 2020

Oh, I found that we can control non-option behavior using the short option string prefix ("+"). Please check review/getopt-v2.

from the man page of getopt_long(3):

By default, getopt() permutes the contents of argv as it scans, so that eventually all the nonoptions are at the end. Two other modes are also implemented. If the first character of optstring is '+' or the environment variable POSIXLY_CORRECT is set, then option processing stops as soon as a nonoption argument is encountered. If the first character of optstring is '-', then each nonoption argv-element is handled as if it were the argument of an option with character code 1. (This is used by programs that were written to expect options and other argv-elements in any order and that care about the ordering of the two.) The special argument "--" forces an end of option-scanning regardless of the scanning mode.

@honggyukim
Copy link
Collaborator

I tested and confirmed that it works fine now. Thanks a lot for your work!

@namhyung
Copy link
Owner Author

namhyung commented Apr 7, 2020

Thanks for the testing!

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

No branches or pull requests

3 participants