Re: [PATCH 4/5] perf tools: Add support for exclusive option
Hi Arnaldo,
On Thu, 23 Oct 2014 17:29:48 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:
>> (2014/10/23 0:15), Namhyung Kim wrote:
>> > Some options cannot be used at the same time. To handle such options
>> > add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
>> > one of them is used.
>>
>> Looks useful for me :)
>>
>> Reviewed-by: Masami Hiramatsu
>>
>> I just have a comment below;
>>
>> > @@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>> >}
>> >
>> >if (arg[1] != '-') {
>> > - ctx->opt = arg + 1;
>> > + ctx->opt = ++arg;
>> >if (internal_help && *ctx->opt == 'h')
>> >return usage_with_options_internal(usagestr,
>> > options, 0);
>> >switch (parse_short_opt(ctx, options)) {
>> >case -1:
>> > - return parse_options_usage(usagestr, options,
>> > arg + 1, 1);
>> > + return parse_options_usage(usagestr, options,
>> > arg, 1);
>> >case -2:
>> >goto unknown;
>> > + case -3:
>> > + goto exclusive;
>>
>> BTW, it may be a time to define return error codes.
>
> Yeah, this thing looks unnecessarily complicated :-\
OK, will do it as a separate patch later.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] perf tools: Add support for exclusive option
Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:
> (2014/10/23 0:15), Namhyung Kim wrote:
> > Some options cannot be used at the same time. To handle such options
> > add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
> > one of them is used.
>
> Looks useful for me :)
>
> Reviewed-by: Masami Hiramatsu
>
> I just have a comment below;
>
> > @@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
> > }
> >
> > if (arg[1] != '-') {
> > - ctx->opt = arg + 1;
> > + ctx->opt = ++arg;
> > if (internal_help && *ctx->opt == 'h')
> > return usage_with_options_internal(usagestr,
> > options, 0);
> > switch (parse_short_opt(ctx, options)) {
> > case -1:
> > - return parse_options_usage(usagestr, options,
> > arg + 1, 1);
> > + return parse_options_usage(usagestr, options,
> > arg, 1);
> > case -2:
> > goto unknown;
> > + case -3:
> > + goto exclusive;
>
> BTW, it may be a time to define return error codes.
Yeah, this thing looks unnecessarily complicated :-\
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] perf tools: Add support for exclusive option
Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:
(2014/10/23 0:15), Namhyung Kim wrote:
Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.
Looks useful for me :)
Reviewed-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
I just have a comment below;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}
if (arg[1] != '-') {
- ctx-opt = arg + 1;
+ ctx-opt = ++arg;
if (internal_help *ctx-opt == 'h')
return usage_with_options_internal(usagestr,
options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options,
arg + 1, 1);
+ return parse_options_usage(usagestr, options,
arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
BTW, it may be a time to define return error codes.
Yeah, this thing looks unnecessarily complicated :-\
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] perf tools: Add support for exclusive option
Hi Arnaldo,
On Thu, 23 Oct 2014 17:29:48 -0300, Arnaldo Carvalho de Melo wrote:
Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:
(2014/10/23 0:15), Namhyung Kim wrote:
Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.
Looks useful for me :)
Reviewed-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
I just have a comment below;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}
if (arg[1] != '-') {
- ctx-opt = arg + 1;
+ ctx-opt = ++arg;
if (internal_help *ctx-opt == 'h')
return usage_with_options_internal(usagestr,
options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options,
arg + 1, 1);
+ return parse_options_usage(usagestr, options,
arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
BTW, it may be a time to define return error codes.
Yeah, this thing looks unnecessarily complicated :-\
OK, will do it as a separate patch later.
Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] perf tools: Add support for exclusive option
(2014/10/23 0:15), Namhyung Kim wrote:
> Some options cannot be used at the same time. To handle such options
> add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
> one of them is used.
Looks useful for me :)
Reviewed-by: Masami Hiramatsu
I just have a comment below;
> @@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
> }
>
> if (arg[1] != '-') {
> - ctx->opt = arg + 1;
> + ctx->opt = ++arg;
> if (internal_help && *ctx->opt == 'h')
> return usage_with_options_internal(usagestr,
> options, 0);
> switch (parse_short_opt(ctx, options)) {
> case -1:
> - return parse_options_usage(usagestr, options,
> arg + 1, 1);
> + return parse_options_usage(usagestr, options,
> arg, 1);
> case -2:
> goto unknown;
> + case -3:
> + goto exclusive;
BTW, it may be a time to define return error codes.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] perf tools: Add support for exclusive option
Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.
Cc: Masami Hiramatsu
Cc: Hemant Kumar
Signed-off-by: Namhyung Kim
---
tools/perf/util/parse-options.c | 59 +
tools/perf/util/parse-options.h | 2 ++
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index b6016101b40b..f62dee7bd924 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -45,6 +45,23 @@ static int get_value(struct parse_opt_ctx_t *p,
if (opt->flags & PARSE_OPT_DISABLED)
return opterror(opt, "is not usable", flags);
+ if (opt->flags & PARSE_OPT_EXCLUSIVE) {
+ if (p->excl_opt) {
+ char msg[128];
+
+ if (((flags & OPT_SHORT) && p->excl_opt->short_name) ||
+ p->excl_opt->long_name == NULL) {
+ scnprintf(msg, sizeof(msg), "cannot be used
with switch `%c'",
+ p->excl_opt->short_name);
+ } else {
+ scnprintf(msg, sizeof(msg), "cannot be used
with %s",
+ p->excl_opt->long_name);
+ }
+ opterror(opt, msg, flags);
+ return -3;
+ }
+ p->excl_opt = opt;
+ }
if (!(flags & OPT_SHORT) && p->opt) {
switch (opt->type) {
case OPTION_CALLBACK:
@@ -345,13 +362,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const char * const usagestr[])
{
int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+ int excl_short_opt = 1;
+ const char *arg;
/* we must reset ->opt, unknown short option leave it dangling */
ctx->opt = NULL;
for (; ctx->argc; ctx->argc--, ctx->argv++) {
- const char *arg = ctx->argv[0];
-
+ arg = ctx->argv[0];
if (*arg != '-' || !arg[1]) {
if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
break;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}
if (arg[1] != '-') {
- ctx->opt = arg + 1;
+ ctx->opt = ++arg;
if (internal_help && *ctx->opt == 'h')
return usage_with_options_internal(usagestr,
options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options,
arg + 1, 1);
+ return parse_options_usage(usagestr, options,
arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
if (ctx->opt)
- check_typos(arg + 1, options);
+ check_typos(arg, options);
while (ctx->opt) {
if (internal_help && *ctx->opt == 'h')
return
usage_with_options_internal(usagestr, options, 0);
@@ -389,6 +409,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx->argv[0] = strdup(ctx->opt - 1);
*(char *)ctx->argv[0] = '-';
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
@@ -404,19 +426,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
break;
}
- if (internal_help && !strcmp(arg + 2, "help-all"))
+ arg += 2;
+ if (internal_help && !strcmp(arg, "help-all"))
return usage_with_options_internal(usagestr, options,
1);
- if (internal_help && !strcmp(arg + 2, "help"))
+ if (internal_help && !strcmp(arg, "help"))
return usage_with_options_internal(usagestr, options,
0);
- if (!strcmp(arg + 2, "list-opts"))
+ if (!strcmp(arg, "list-opts"))
return PARSE_OPT_LIST_OPTS;
- if (!strcmp(arg + 2, "list-cmds"))
+ if (!strcmp(arg, "list-cmds"))
return PARSE_OPT_LIST_SUBCMDS;
-
Re: [PATCH 4/5] perf tools: Add support for exclusive option
(2014/10/23 0:15), Namhyung Kim wrote:
Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.
Looks useful for me :)
Reviewed-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
I just have a comment below;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}
if (arg[1] != '-') {
- ctx-opt = arg + 1;
+ ctx-opt = ++arg;
if (internal_help *ctx-opt == 'h')
return usage_with_options_internal(usagestr,
options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options,
arg + 1, 1);
+ return parse_options_usage(usagestr, options,
arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
BTW, it may be a time to define return error codes.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] perf tools: Add support for exclusive option
Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Hemant Kumar hem...@linux.vnet.ibm.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/util/parse-options.c | 59 +
tools/perf/util/parse-options.h | 2 ++
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index b6016101b40b..f62dee7bd924 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -45,6 +45,23 @@ static int get_value(struct parse_opt_ctx_t *p,
if (opt-flags PARSE_OPT_DISABLED)
return opterror(opt, is not usable, flags);
+ if (opt-flags PARSE_OPT_EXCLUSIVE) {
+ if (p-excl_opt) {
+ char msg[128];
+
+ if (((flags OPT_SHORT) p-excl_opt-short_name) ||
+ p-excl_opt-long_name == NULL) {
+ scnprintf(msg, sizeof(msg), cannot be used
with switch `%c',
+ p-excl_opt-short_name);
+ } else {
+ scnprintf(msg, sizeof(msg), cannot be used
with %s,
+ p-excl_opt-long_name);
+ }
+ opterror(opt, msg, flags);
+ return -3;
+ }
+ p-excl_opt = opt;
+ }
if (!(flags OPT_SHORT) p-opt) {
switch (opt-type) {
case OPTION_CALLBACK:
@@ -345,13 +362,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const char * const usagestr[])
{
int internal_help = !(ctx-flags PARSE_OPT_NO_INTERNAL_HELP);
+ int excl_short_opt = 1;
+ const char *arg;
/* we must reset -opt, unknown short option leave it dangling */
ctx-opt = NULL;
for (; ctx-argc; ctx-argc--, ctx-argv++) {
- const char *arg = ctx-argv[0];
-
+ arg = ctx-argv[0];
if (*arg != '-' || !arg[1]) {
if (ctx-flags PARSE_OPT_STOP_AT_NON_OPTION)
break;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}
if (arg[1] != '-') {
- ctx-opt = arg + 1;
+ ctx-opt = ++arg;
if (internal_help *ctx-opt == 'h')
return usage_with_options_internal(usagestr,
options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options,
arg + 1, 1);
+ return parse_options_usage(usagestr, options,
arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
if (ctx-opt)
- check_typos(arg + 1, options);
+ check_typos(arg, options);
while (ctx-opt) {
if (internal_help *ctx-opt == 'h')
return
usage_with_options_internal(usagestr, options, 0);
@@ -389,6 +409,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx-argv[0] = strdup(ctx-opt - 1);
*(char *)ctx-argv[0] = '-';
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
@@ -404,19 +426,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
break;
}
- if (internal_help !strcmp(arg + 2, help-all))
+ arg += 2;
+ if (internal_help !strcmp(arg, help-all))
return usage_with_options_internal(usagestr, options,
1);
- if (internal_help !strcmp(arg + 2, help))
+ if (internal_help !strcmp(arg, help))
return usage_with_options_internal(usagestr, options,
0);
- if (!strcmp(arg + 2, list-opts))
+ if (!strcmp(arg, list-opts))
return PARSE_OPT_LIST_OPTS;
- if (!strcmp(arg + 2, list-cmds))
+ if (!strcmp(arg, list-cmds))
return PARSE_OPT_LIST_SUBCMDS;
-
