From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1EWKLr-0001Pb-9S for mharc-grub-devel@gnu.org; Sun, 30 Oct 2005 16:04:27 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1EWKLp-0001PN-FQ for grub-devel@gnu.org; Sun, 30 Oct 2005 16:04:25 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1EWKLn-0001PA-S0 for grub-devel@gnu.org; Sun, 30 Oct 2005 16:04:25 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1EWKLn-0001P7-PH for grub-devel@gnu.org; Sun, 30 Oct 2005 16:04:23 -0500 Received: from [145.74.66.11] (helo=mail-cn.han.nl) by monty-python.gnu.org with esmtp (Exim 4.34) id 1EWKLn-0008AA-EJ for grub-devel@gnu.org; Sun, 30 Oct 2005 16:04:23 -0500 Received: from vscan-cn.han.nl (venus.han.nl [145.74.65.6]) by mail-cn.han.nl (Postfix) with ESMTP id 3F2188974 for ; Sun, 30 Oct 2005 22:04:22 +0100 (CET) Received: from mail-cn.han.nl ([145.74.66.11]) by vscan-cn.han.nl (venus.han.nl [145.74.65.6]) (amavisd-new, port 10024) with ESMTP id 29332-02 for ; Sun, 30 Oct 2005 22:04:21 +0100 (CET) Received: from mail1.han.nl (mail1.han.nl [145.74.103.11]) by mail-cn.han.nl (Postfix) with ESMTP id 5F9C98875 for ; Sun, 30 Oct 2005 22:04:21 +0100 (CET) Received: from localhost.localdomain (mgerards.xs4all.nl [82.92.27.129]) by mail1.han.nl (Postfix) with ESMTP id E1776C047 for ; Sun, 30 Oct 2005 22:04:20 +0100 (CET) Mail-Copies-To: metgerards@student.han.nl To: The development of GRUB 2 References: <87u0f0t30k.fsf@student.han.nl> From: Marco Gerards Date: Sun, 30 Oct 2005 22:04:22 +0100 In-Reply-To: (Hollis Blanchard's message of "Sun, 30 Oct 2005 14:49:31 -0600") Message-ID: <871x22hg2x.fsf@student.han.nl> User-Agent: Gnus/5.1007 (Gnus v5.10.7) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Virus-Scanned: by amavisd-new (2.2.0) at vscan-cn.han.nl Subject: Re: Scripting support (PATCH) X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 30 Oct 2005 21:04:25 -0000 Hollis Blanchard writes: > I don't have many comments, since I'm not familiar with lexing and > parsing. Just a couple small comments: > > On Oct 29, 2005, at 4:41 PM, Marco Gerards wrote: >> >> `if' accepts any command. I showed the example of using the `[' >> command. It can be made similar to the `[' command of coreutils. >> This command sets the environment variable `RESULT' to either 0 or 1. >> After execution `if' checks `RESULT' and either executes the `if' or >> the `else' branch. It would also be possible to use a return value >> instead of `RESULT', but that was too much trouble for me at first. >> But of course this can be changed if someone can convince me of that. > > You want it to be "$RESULT" instead of "$?" ? You are right. > I really don't like that each command has to explicitly set RESULT. As > you note, it would be better if the return code from the command were > automatically placed into the status environment variable. Most command return grub_err_t. The only commands that matter for us are commands like `['. Would you propose every commands returns an int and that on function return grub_errno is checked? >> +#ifdef GRUB_UTIL >> +void >> +grub_lsb_init (void) >> +{ >> + grub_register_command ("[", grub_cmd_lsb, GRUB_COMMAND_FLAG_CMDLINE, >> + "[ EXPRESSION ]", "Evaluate an expression", 0); >> +} >> + >> +void >> +grub_lsb_fini (void) >> +{ >> + grub_unregister_command ("["); >> +} >> +#else /* ! GRUB_UTIL */ >> +GRUB_MOD_INIT >> +{ >> + (void)mod; /* To stop warning. */ >> + grub_register_command ("[", grub_cmd_lsb, GRUB_COMMAND_FLAG_CMDLINE, >> + "[ EXPRESSION ]", "Evaluate an expression", 0); >> +} >> + >> +GRUB_MOD_FINI >> +{ >> + grub_unregister_command ("["); >> +} >> +#endif /* ! GRUB_UTIL */ > > We *really* need to redefine GRUB_MOD_INIT/FINI to remove all this > duplicated code. I guess I will add that to my list. Cool :-) >> +/* A part of an argument. */ >> +struct grub_script_arg >> +{ >> + /* If this is 0, STR is a string. If it is one, STR is a variable >> + name. */ >> + int type; > > This should probably be an enum. Yes, good catch. > Since this code won't break any existing behavior (simple "ls (hd,0)/" > will still work, right?), I guess it can be committed as soon as > serious issues like the memory leak have been fixed. That is right. All normal code will continue to function, otherwise it would be a bug or have been discussed on the list. So far there is nothing that would break the current behavior except multiple lines (which I should fix before committing). -- Marco