Re: [PATCH 2/2] Allow user-defined functions to override builtins. [take 2]

[Glenn Washburn]

I agree that a documentation change would make this patch better.
However, I don't see it as a hack.  Essentially I've made "builtin" a
contruct of the language, rather than a true command.  I don't see the
need to make it a true command.  I see two reasons why you might want
to do that.

1. You want to hook/override the builtin command.  Why should this be
allowed when it should just be running the builtin and nothing else?

2. You want to make builtin a module that you can choose not to load,
so as not to have the builtin functionality.  What's the point?

On Thu, 22 May 2014 15:59:58 +0200
Michel Hermier <address@hidden> wrote:

> I don't know the code much, but i think your change looks like a hack.
> I fell missing an internal command registration in main.c, a small
> function to handle the command, and an extra parameter in
> grub_script_execute_cmdline to allow/exclude user defined commands so
> that it can be called from builtin, and a change in the documentation
> to advertise about builtin command.
> Cheers
> Le 22 mai 2014 08:46, "Glenn Washburn" <address@hidden> a
> écrit :
> 
> > This is implements the builtin pseudo-command.
> >
> > ---
> >  grub-core/script/execute.c | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/grub-core/script/execute.c b/grub-core/script/execute.c
> > index 857f8c6..2dbc17f 100644
> > --- a/grub-core/script/execute.c
> > +++ b/grub-core/script/execute.c
> > @@ -917,6 +917,7 @@ grub_script_execute_cmdline (struct
> > grub_script_cmd *cmd) int argc;
> >    char **args;
> >    int invert;
> > +  int builtin;
> >    struct grub_script_argv argv = { 0, 0, 0 };
> >
> >    /* Lookup the command.  */
> > @@ -924,6 +925,7 @@ grub_script_execute_cmdline (struct
> > grub_script_cmd *cmd) return grub_errno;
> >
> >    invert = 0;
> > +  builtin = 0;
> >    argc = argv.argc - 1;
> >    args = argv.args + 1;
> >    cmdname = argv.args[0];
> > @@ -937,12 +939,27 @@ grub_script_execute_cmdline (struct
> > grub_script_cmd *cmd) }
> >
> >        invert = 1;
> > -      argc = argv.argc - 2;
> > -      args = argv.args + 2;
> > -      cmdname = argv.args[1];
> > +      cmdname = args[0];
> > +      argc--;
> > +      args++;
> > +    }
> > +  if (grub_strcmp (cmdname, "builtin") == 0)
> > +    {
> > +      if (argv.argc < 2 || ! argv.args[1])
> > +       {
> > +         grub_script_argv_free (&argv);
> > +         return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > +                            N_("no command is specified"));
> > +       }
> > +
> > +      builtin = 1;
> > +      cmdname = args[0];
> > +      argc--;
> > +      args++;
> >      }
> >    /* Allow user functions to override built in commands. */
> > -  func = grub_script_function_find (cmdname);
> > +  if (! builtin)
> > +    func = grub_script_function_find (cmdname);
> >    if (! func)
> >      {
> >        grub_errno = GRUB_ERR_NONE;
> > --
> > 1.8.3.2
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >