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

Discussion on Options and Parameters in Proxmark Commands #467

Closed
pwpiwi opened this issue Nov 10, 2017 · 24 comments
Closed

Discussion on Options and Parameters in Proxmark Commands #467

pwpiwi opened this issue Nov 10, 2017 · 24 comments

Comments

@pwpiwi
Copy link
Contributor

pwpiwi commented Nov 10, 2017

@iceman1001 wrote in #465:

Pretty soon the discussion of a unified parameter is needed, since you seem to be fond of the prefix "-"
while we have spend time not using it. I don't mind either, but I prefer that we have one and not three like >we have to day.

no prefixed inparameters.
letter only prefixed parameters
"-" + letter prefixed parameters.
Same goes for inputed "hex".. Either "405060" or "40 50 60" or both accepted.

This is indeed a topic I wanted to discuss ages ago. The proxmark way of handling parameters and options is neither consistent nor common practice. We are all used to options like -h and an alternative long version like --help for any Unix or Windows program. Why don't we adopt this common practice and start using getopt() (or its sister getopt_long()) as in client\loclass\main.c and client\reveng\cli.c? This is not only common practice but also avoids those ugly command line parsing we have in many places.

@iceman1001
Copy link
Member

I welcome this.

LUA script also uses "-" + letter prefix
All different command line parsing is ineffective.

I just one way, not three. And in a standard way so its easy for new contributors to add. Not to worry or spend time in "how to parse commandline in proxmark".

@marshmellow42
Copy link
Contributor

i'm for it. ( though i will have to learn how getopt() works.. ;) )

@iceman1001
Copy link
Member

yeah, getopt() crash-course needed for me too.

@pwpiwi
Copy link
Contributor Author

pwpiwi commented Nov 10, 2017

@iceman1001
Copy link
Member

Actually, all blogs, forum posts will be upset but I still want it to be unified.

@iceman1001
Copy link
Member

...well... piwi... will you start the unifying with one getopt? The loclass, reveng has their own, we would need to have one.

@merlokk
Copy link
Contributor

merlokk commented Nov 10, 2017

So, it needs to use getopt_long?

and this?
"-" + letter prefixed parameters. (dont know... lets think)
Same goes for imputed "hex".. "405060" and "40 50 60" accepted. (it will be cool because some strings in the internet with spaces, some without.... just for more convenient copy-paste)

@iceman1001
Copy link
Member

getopt_long, whats the difference?

if we have [-h] , which could be confusing between being in the shell vs inside pm3 client.. What you think?

Agreed, hex input both kinds, with/ wo spaces, would increase usability.

@merlokk
Copy link
Contributor

merlokk commented Nov 11, 2017

I don't know what the difference between getopt libs. Ill look.
We need functionality like:

bool SelectField = OptionExists("s");
GetOptionParam("a", APDU, &APDUlen);
GetLastParam(APDU, &APDUlen);

@merlokk
Copy link
Contributor

merlokk commented Nov 11, 2017

I have time to investigate at first part of next week. If someone can do earlier, please, do)

@iceman1001
Copy link
Member

iceman1001 commented Nov 11, 2017

I'm guessing,

getopt ==  "-h"
getopt_long ==  "--help"


@merlokk
Copy link
Contributor

merlokk commented Nov 13, 2017

https://github.com/skeeto/optparse
https://github.com/cofyc/argparse

need too look - maybe there is cool parser on github)

@pwpiwi
Copy link
Contributor Author

pwpiwi commented Nov 14, 2017

skeeto makes some good points in his README.md (https://github.com/skeeto/optparse/blob/master/README.me) which discourage the usage of the original getopt(). cofyc prevents the same drawbacks and adds error message handling. Both licenses are compatible with GPL v2.

+1 for cofyc/argparse

@merlokk
Copy link
Contributor

merlokk commented Nov 14, 2017

@merlokk
Copy link
Contributor

merlokk commented Nov 14, 2017

and all of them dont have initialization from char*, just from argc argv

@pwpiwi
Copy link
Contributor Author

pwpiwi commented Nov 14, 2017

Yes, a function to split the line into options and parameters is required in all cases. It would be an additipnal task of this function to distinguish a hex parameter with spaces from multiple parameters.

@merlokk
Copy link
Contributor

merlokk commented Nov 14, 2017

If we have - at the parameter's beginning - all is ok, this function can be written easily)
If not - needs to think.

guys, what are you thinking about parameters: - or without?

@pwpiwi
Copy link
Contributor Author

pwpiwi commented Nov 14, 2017

I am for the standard: - or -- for options. = or nothing for parameters. Nothing for additional arguments.

HF 14a raw -c -t 10 60 01

HF 14a raw : command
-c : option
-t: option with mandatory parameter
10: parameter
60 01 : arguments

@merlokk
Copy link
Contributor

merlokk commented Nov 14, 2017

and HF 14a raw -ct 10 60 01 too ?

@merlokk
Copy link
Contributor

merlokk commented Nov 16, 2017

I look at https://github.com/argtable/argtable3
and see that it may be what we need:

    struct arg_lit *n     = arg_lit0("n", NULL,         "do not output the trailing newline");
    struct arg_lit *e     = arg_lit0("e", NULL,         "enable interpretation of the backslash-escaped characters listed below");
    struct arg_lit *E     = arg_lit0("E", NULL,         "disable interpretation of those sequences in <string>s");
    struct arg_lit *help  = arg_lit0(NULL,"help",       "print this help and exit");
    struct arg_str *strs  = arg_strn(NULL,NULL,"STRING",0,argc+2,NULL);
    struct arg_end *end   = arg_end(20);
    void* argtable[] = {n,e,E,help,vers,strs,end};

I will try to implement one command
OK?

@pwpiwi
Copy link
Contributor Author

pwpiwi commented Nov 16, 2017

Looks great. Has a good tutorial as well: http://www.argtable.org/tutorial/

I propose to start with hf 14a raw. It should be possible to refactor it without changing the syntax.

@merlokk
Copy link
Contributor

merlokk commented Nov 17, 2017

OK, i will try and show PR) lets look...

@merlokk
Copy link
Contributor

merlokk commented Nov 26, 2017

OK. lets think, test and merge.

especially needs to check if reveng part still works OK.

@merlokk
Copy link
Contributor

merlokk commented Sep 28, 2018

can be closed

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

No branches or pull requests

4 participants