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

fix iovisor/bcc/issues/#1851 for Arch Linux users #173

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

Conversation

NitroCao
Copy link

The fix for Arch Linux came from iovisor/bcc#1851.

Have tested that the script can run on Ubuntu 18.04.2 without any modification.
Haven't tested for other distributions.

Copy link
Collaborator

@schu schu left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. Besides the inline comments, can you maybe rename the example to e.g. uprobe_readline since it's not bash specific anymore per se with your update?

)

func init() {
flag.BoolVar(&h, "h", false, "Print this help message")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The flag package does print help text by default for -h / --help, so that's not needed I think.

os.Exit(1)
}
if name == "" {
name = "/bin/bash"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can set the default directly with flag.StringVar.


func init() {
flag.BoolVar(&h, "h", false, "Print this help message")
flag.StringVar(&name, "s", "", `Specify the location of libreadline.so library.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of s, please give the flag a more descriptive name, e.g. target.

flag.BoolVar(&h, "h", false, "Print this help message")
flag.StringVar(&name, "s", "", `Specify the location of libreadline.so library.
You may need specify this option when failing to run the script directly.`)
flag.Usage = usage
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just go with the default usage message from the flag package.

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