Re: New commands (reboot, halt, help)

[Yoshinori K. Okuji]

On Friday 28 January 2005 13:36, Marco Gerards wrote:
> +static int
> +print_command_info (grub_command_t cmd)
> +{
> +  if (cmd->flags & GRUB_COMMAND_FLAG_CMDLINE
> +      || cmd->flags & GRUB_COMMAND_FLAG_BOTH)
> +    grub_printf ("%s\t\t%s\n", cmd->name, cmd->description);
> +  return 0;
> +}

Why do you test the flags with GRUB_COMMAND_FLAG_BOTH?

BTW, I prefer a two-column format like GRUB Legacy and BASH. Otherwise, 
it can emit too many lines.

> +static grub_err_t
> +grub_cmd_help (struct grub_arg_list *state __attribute__ ((unused)),
> +            int argc __attribute__ ((unused)),
> +            char **args __attribute__ ((unused)))
> +
> +{
> +  grub_printf ("All GRUB commands accept `--help'.\n\n");
> +
> +  grub_iterate_commands (print_command_info);
> +  return 0;
> +}

Please make it possible to specify arguments (e.g help ls).

> +  grub_register_command ("help", grub_cmd_help,
> GRUB_COMMAND_FLAG_BOTH, +                      "help", "Shows a help 
> message", 0);
> +}

I think GRUB_COMMAND_FLAG_CMDLINE should be used for this command. I 
cannot think of any case where help is useful in the menu interface.

> EXPORT_FUNC(grub_register_command)
>  void EXPORT_FUNC(grub_unregister_command) (const char *name);
>  grub_command_t grub_command_find (char *cmdline);
>  grub_err_t grub_set_history (int newsize);
> -int grub_iterate_commands (int (*iterate) (grub_command_t));
> +int EXPORT_FUNC(grub_iterate_commands) (int (*iterate)
> (grub_command_t)); int grub_command_execute (char *cmdline);
>  void grub_command_init (void);
>  void grub_normal_init_page (void);

This is strange. You don't have to specify EXPORT_FUNC in normal.h, 
because the function is defined in a module but not in a kernel. 
EXPORT_* are required only by the kernel (because the kernel may not be 
an ELF object).

Okuji