Patchwork [08/16] qemu-io: Move 'help' function

login
register
mail settings
Submitter Kevin Wolf
Date May 28, 2013, 3:27 p.m.
Message ID <1369754856-30036-9-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/246901/
State New
Headers show

Comments

Kevin Wolf - May 28, 2013, 3:27 p.m.
No reason to treat it different from other commands. Move it to
qemu-io-cmds.c, adapt the coding style and register it like any other
command.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 cmd.c          | 79 ----------------------------------------------------------
 cmd.h          |  1 -
 qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 66 insertions(+), 81 deletions(-)
Eric Blake - May 29, 2013, 9:25 p.m.
On 05/28/2013 09:27 AM, Kevin Wolf wrote:
> No reason to treat it different from other commands. Move it to
> qemu-io-cmds.c, adapt the coding style and register it like any other
> command.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  cmd.c          | 79 ----------------------------------------------------------
>  cmd.h          |  1 -
>  qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 66 insertions(+), 81 deletions(-)

> -void
> -help_init(void)
> -{
> -	help_cmd.name = _("help");
> -	help_cmd.altname = _("?");
> -	help_cmd.cfunc = help_f;
> -	help_cmd.argmin = 0;
> -	help_cmd.argmax = 1;
> -	help_cmd.flags = CMD_FLAG_GLOBAL;
> -	help_cmd.args = _("[command]");
> -	help_cmd.oneline = _("help for one or all commands");

The old code marked strings for translation with _(), ...

> +static const cmdinfo_t help_cmd = {
> +    .name       = "help",
> +    .altname    = "?",
> +    .cfunc      = help_f,
> +    .argmin     = 0,
> +    .argmax     = 1,
> +    .flags      = CMD_FLAG_GLOBAL,
> +    .args       = "[command]",
> +    .oneline    = "help for one or all commands",

...whereas the new code uses fixed literals.  Then again, po/messages.po
doesn't contain any mention of cmd.c, so it's not like we were actually
translating the strings, even if they were marked for translation.  So
no real change.

Reviewed-by: Eric Blake <eblake@redhat.com>
Kevin Wolf - June 4, 2013, 10:20 a.m.
Am 29.05.2013 um 23:25 hat Eric Blake geschrieben:
> On 05/28/2013 09:27 AM, Kevin Wolf wrote:
> > No reason to treat it different from other commands. Move it to
> > qemu-io-cmds.c, adapt the coding style and register it like any other
> > command.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  cmd.c          | 79 ----------------------------------------------------------
> >  cmd.h          |  1 -
> >  qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  3 files changed, 66 insertions(+), 81 deletions(-)
> 
> > -void
> > -help_init(void)
> > -{
> > -	help_cmd.name = _("help");
> > -	help_cmd.altname = _("?");
> > -	help_cmd.cfunc = help_f;
> > -	help_cmd.argmin = 0;
> > -	help_cmd.argmax = 1;
> > -	help_cmd.flags = CMD_FLAG_GLOBAL;
> > -	help_cmd.args = _("[command]");
> > -	help_cmd.oneline = _("help for one or all commands");
> 
> The old code marked strings for translation with _(), ...
> 
> > +static const cmdinfo_t help_cmd = {
> > +    .name       = "help",
> > +    .altname    = "?",
> > +    .cfunc      = help_f,
> > +    .argmin     = 0,
> > +    .argmax     = 1,
> > +    .flags      = CMD_FLAG_GLOBAL,
> > +    .args       = "[command]",
> > +    .oneline    = "help for one or all commands",
> 
> ...whereas the new code uses fixed literals.  Then again, po/messages.po
> doesn't contain any mention of cmd.c, so it's not like we were actually
> translating the strings, even if they were marked for translation.  So
> no real change.

In fact, the real reason is that cmd.c has this line:

  #define _(x)    x   /* not gettext support yet */

Apart from that, translating command names has never been a great idea
anyway, so at least that part wouldn't a big loss even if gettext was
actually used...

Kevin
Stefan Hajnoczi - June 5, 2013, 12:37 p.m.
On Tue, May 28, 2013 at 05:27:28PM +0200, Kevin Wolf wrote:
> No reason to treat it different from other commands. Move it to
> qemu-io-cmds.c, adapt the coding style and register it like any other
> command.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  cmd.c          | 79 ----------------------------------------------------------
>  cmd.h          |  1 -
>  qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 66 insertions(+), 81 deletions(-)
> 
> diff --git a/cmd.c b/cmd.c
> index 7ae978f..2941ad3 100644
> --- a/cmd.c
> +++ b/cmd.c

'help' is a generic command, it's not specific to qemu-io.  IMO cmd.c is
the right place for it since other command-line tools might also need
'help'.

Patch

diff --git a/cmd.c b/cmd.c
index 7ae978f..2941ad3 100644
--- a/cmd.c
+++ b/cmd.c
@@ -439,82 +439,3 @@  quit_init(void)
 
 	add_command(&quit_cmd);
 }
-
-/* from libxcmd/help.c */
-
-static cmdinfo_t help_cmd;
-static void help_onecmd(const char *cmd, const cmdinfo_t *ct);
-static void help_oneline(const char *cmd, const cmdinfo_t *ct);
-
-static void
-help_all(void)
-{
-	const cmdinfo_t	*ct;
-
-	for (ct = cmdtab; ct < &cmdtab[ncmds]; ct++)
-		help_oneline(ct->name, ct);
-	printf(_("\nUse 'help commandname' for extended help.\n"));
-}
-
-static int
-help_f(
-    BlockDriverState *bs,
-	int		argc,
-	char		**argv)
-{
-	const cmdinfo_t	*ct;
-
-	if (argc == 1) {
-		help_all();
-		return 0;
-	}
-	ct = find_command(argv[1]);
-	if (ct == NULL) {
-		printf(_("command %s not found\n"), argv[1]);
-		return 0;
-	}
-	help_onecmd(argv[1], ct);
-	return 0;
-}
-
-static void
-help_onecmd(
-	const char	*cmd,
-	const cmdinfo_t	*ct)
-{
-	help_oneline(cmd, ct);
-	if (ct->help)
-		ct->help();
-}
-
-static void
-help_oneline(
-	const char	*cmd,
-	const cmdinfo_t	*ct)
-{
-	if (cmd)
-		printf("%s ", cmd);
-	else {
-		printf("%s ", ct->name);
-		if (ct->altname)
-			printf("(or %s) ", ct->altname);
-	}
-	if (ct->args)
-		printf("%s ", ct->args);
-	printf("-- %s\n", ct->oneline);
-}
-
-void
-help_init(void)
-{
-	help_cmd.name = _("help");
-	help_cmd.altname = _("?");
-	help_cmd.cfunc = help_f;
-	help_cmd.argmin = 0;
-	help_cmd.argmax = 1;
-	help_cmd.flags = CMD_FLAG_GLOBAL;
-	help_cmd.args = _("[command]");
-	help_cmd.oneline = _("help for one or all commands");
-
-	add_command(&help_cmd);
-}
diff --git a/cmd.h b/cmd.h
index d676408..89e7c6e 100644
--- a/cmd.h
+++ b/cmd.h
@@ -42,7 +42,6 @@  typedef struct cmdinfo {
 extern cmdinfo_t	*cmdtab;
 extern int		ncmds;
 
-void help_init(void);
 void quit_init(void);
 
 typedef int (*checkfunc_t)(BlockDriverState *bs, const cmdinfo_t *ci);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index bdfe5f2..e2d65d7 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1795,6 +1795,71 @@  static const cmdinfo_t abort_cmd = {
        .oneline        = "simulate a program crash using abort(3)",
 };
 
+static void help_oneline(const char *cmd, const cmdinfo_t *ct)
+{
+    if (cmd) {
+        printf("%s ", cmd);
+    } else {
+        printf("%s ", ct->name);
+        if (ct->altname) {
+            printf("(or %s) ", ct->altname);
+        }
+    }
+
+    if (ct->args) {
+        printf("%s ", ct->args);
+    }
+    printf("-- %s\n", ct->oneline);
+}
+
+static void help_onecmd(const char *cmd, const cmdinfo_t *ct)
+{
+    help_oneline(cmd, ct);
+    if (ct->help) {
+        ct->help();
+    }
+}
+
+static void help_all(void)
+{
+    const cmdinfo_t *ct;
+
+    for (ct = cmdtab; ct < &cmdtab[ncmds]; ct++) {
+        help_oneline(ct->name, ct);
+    }
+    printf("\nUse 'help commandname' for extended help.\n");
+}
+
+static int help_f(BlockDriverState *bs, int argc, char **argv)
+{
+    const cmdinfo_t *ct;
+
+    if (argc == 1) {
+        help_all();
+        return 0;
+    }
+
+    ct = find_command(argv[1]);
+    if (ct == NULL) {
+        printf("command %s not found\n", argv[1]);
+        return 0;
+    }
+
+    help_onecmd(argv[1], ct);
+    return 0;
+}
+
+static const cmdinfo_t help_cmd = {
+    .name       = "help",
+    .altname    = "?",
+    .cfunc      = help_f,
+    .argmin     = 0,
+    .argmax     = 1,
+    .flags      = CMD_FLAG_GLOBAL,
+    .args       = "[command]",
+    .oneline    = "help for one or all commands",
+};
+
 static int init_check_command(BlockDriverState *bs, const cmdinfo_t *ct)
 {
     if (ct->flags & CMD_FLAG_GLOBAL) {
@@ -1834,7 +1899,7 @@  bool qemuio_command(const char *cmd)
 static void __attribute((constructor)) init_qemuio_commands(void)
 {
     /* initialize commands */
-    help_init();
+    add_command(&help_cmd);
     add_command(&read_cmd);
     add_command(&readv_cmd);
     add_command(&write_cmd);