From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1Ltkyy-0000kO-HD for mharc-grub-devel@gnu.org; Tue, 14 Apr 2009 11:55:32 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ltkyx-0000jq-3S for grub-devel@gnu.org; Tue, 14 Apr 2009 11:55:31 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ltkyw-0000jL-CU for grub-devel@gnu.org; Tue, 14 Apr 2009 11:55:30 -0400 Received: from [199.232.76.173] (port=47841 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ltkyw-0000jC-6I for grub-devel@gnu.org; Tue, 14 Apr 2009 11:55:30 -0400 Received: from mail.nexedi.com ([91.121.25.85]:43882 helo=nexedi.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ltkyv-0004zY-Eu for grub-devel@gnu.org; Tue, 14 Apr 2009 11:55:29 -0400 Received: from [10.8.0.46] (unknown [10.8.0.46]) by nexedi.com (Postfix) with ESMTP id 18D7D3E0DF for ; Tue, 14 Apr 2009 17:55:27 +0200 (CEST) From: "Yoshinori K. Okuji" Organization: enbug.org To: The development of GRUB 2 Date: Wed, 15 Apr 2009 00:55:36 +0900 User-Agent: KMail/1.9.10 References: <49955BD7.2070206@gmail.com> <200904111907.08977.okuji@enbug.org> <49E0B331.4010305@gmail.com> In-Reply-To: <49E0B331.4010305@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200904150055.36476.okuji@enbug.org> X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 3) Subject: Re: [PATCH] Test command 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, 14 Apr 2009 15:55:31 -0000 On Sunday 12 April 2009 00:11:45 phcoder wrote: > Updated. Same changelog > > >> + { > >> + update_val (grub_strcmp (args[*argn], args[*argn + 2]) == 0); > >> + (*argn) += 3; > > > > I myself feel that these parentheses are redundant, but I don't know how > > others think. For C programmers, it is well known that * has a very high > > priority. > > These parenthesis are necessary if doing sth like > (*argn)++ > since ++ and += are logically and visually similar I prefer to put the > parenthesis OK. > > > Getting tired, so I skip the same criticism from here. > > Actually it would have been enough to say "same applies further on in > patch" > > >> + if (*argn + 1 < argc && !grub_strcmp (args[*argn], "-s")) > >> + { > >> + grub_file_t file; > >> + file = grub_file_open (args[*argn + 1]); > >> + update_val (file && grub_file_size (file)); > > > > This is not very safe, because grub_file_size returns grub_off_t which is > > a 64-bit unsigned int. By converting it into 32-bit signed int > > implicitly, the result can be zero, even when the size is not zero. So it > > is better to say explicitly, != 0. BTW, I think you can simplify test_parse. For example, you write "if (*argn + 2 < argc ...)" many times, but it should be possible to test this condition only once per loop. Regards, Okuji