From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1HvctO-0001XY-Bb for mharc-grub-devel@gnu.org; Tue, 05 Jun 2007 13:32:26 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1HvctN-0001XC-04 for grub-devel@gnu.org; Tue, 05 Jun 2007 13:32:25 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1HvctL-0001Wf-72 for grub-devel@gnu.org; Tue, 05 Jun 2007 13:32:23 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HvctK-0001Wc-Ub for grub-devel@gnu.org; Tue, 05 Jun 2007 13:32:23 -0400 Received: from 245.red-80-24-127.staticip.rima-tde.net ([80.24.127.245] helo=manazas.ensanjose.net) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1HvctK-0001PX-7C for grub-devel@gnu.org; Tue, 05 Jun 2007 13:32:22 -0400 Received: from localhost (manazas.ensanjose.net [127.0.0.1]) by manazas.ensanjose.net (Postfix) with ESMTP id 8B570D444D for ; Tue, 5 Jun 2007 19:32:19 +0200 (CEST) Received: from manazas.ensanjose.net ([127.0.0.1]) by localhost (manazas.ensanjose.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 07831-05 for ; Tue, 5 Jun 2007 19:32:19 +0200 (CEST) Received: from [192.168.1.8] (163.Red-83-34-181.dynamicIP.rima-tde.net [83.34.181.163]) by manazas.ensanjose.net (Postfix) with ESMTP id 1A239D321B for ; Tue, 5 Jun 2007 19:32:18 +0200 (CEST) Message-ID: <46659E21.5010805@raulete.net> Date: Tue, 05 Jun 2007 19:32:17 +0200 From: adrian15 User-Agent: Mozilla Thunderbird 1.0.7 (Windows/20050923) X-Accept-Language: en-us, en MIME-Version: 1.0 To: grub-devel@gnu.org References: <200706041604.l54G4p0v029961@correoredir01.dinaserver.com> In-Reply-To: <200706041604.l54G4p0v029961@correoredir01.dinaserver.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-detected-kernel: Linux 2.4-2.6 Subject: Re: test -e 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: Tue, 05 Jun 2007 17:32:25 -0000 > adrian15 writes: > >> > Attached you will find the patch adding test -e support for grub2. >> > >> > This is my first patch. I have compiled it without no errors. > > Urgh... I thought/hoped I told you I had a test.c rewrite sitting on > my harddisk? Or did I tell Robert to poke me until next weekend so I > will work on it? It includes everything you expect from test.c, > expect the cleanup and testing. ;-) Do you mean you also have the '-e' option ? > Please have a look at the wiki. It has quite some information about > GRUB 2. Whenever possible I'll download some info from the wiki. >> > Should I write "Test if a file exists" instead of "test if a file >> > exists" or "FILE exists"? > > FILE FILE or FILE exists ? Or have you coded it yourself too? >> > How the hell should I treat grub errors. Maybe the test_file_exists has >> > to return a static grub_err_t and then from grub_cmd_test I should call >> > it like this: return (test_file_exists (args[0])) so that the error >> > propagates ? > > Right. Otherwise the error will be lost. Ok. I will try that every function propagates errors. > >> > What are the error polices ? > > Returning the err_t that grub_error returns when possible. Ok. >> > One question. Should we put the test-e function into a separate file or >> > not ? > > No, the problem is that the design of test.c (which is just a > placeholder) is wrong. It needs a proper parser for the arguments and > a way to deal with this... Ok. We will wait for your code. >> > The question is if the user will see the -e, -f or other options when >> > querying the test command help or not ? > > They should. But I am not sure if the final version will support > this. Especially because of the nested syntax of the test arguments. Do you mean the -e options support or do you mean the -e options showing at help test support ? >> > +static void >> > +test_file_exists (const char *key) > > Why not filename? test_filename_exists or filename ? >> > { >> > + > > You accidently introduced a whiteline. No whitelines after an initial {. I write down it. >> > + if (state[0].set) >> > + test_file_exists (args[0]); >> > + else >> > + { > > This means that this check is run for any other expression. This is > quite error sensitive. In my code the only implemented option is '-e'. When there will be more I could add more nested if with the other options, or maybe better we will enjoy your improved code. adrian15