From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1GcLbo-0001Si-5R for mharc-grub-devel@gnu.org; Tue, 24 Oct 2006 08:42:20 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1GcLbm-0001SN-9G for grub-devel@gnu.org; Tue, 24 Oct 2006 08:42:18 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1GcLbg-0001Og-U3 for grub-devel@gnu.org; Tue, 24 Oct 2006 08:42:17 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1GcLbg-0001Od-Jq for grub-devel@gnu.org; Tue, 24 Oct 2006 08:42:12 -0400 Received: from [195.54.107.76] (helo=mxfep03.bredband.com) by monty-python.gnu.org with esmtp (Exim 4.52) id 1GcLbg-0004CW-CF for grub-devel@gnu.org; Tue, 24 Oct 2006 08:42:12 -0400 Received: from ironport.bredband.com ([195.54.107.82] [195.54.107.82]) by mxfep03.bredband.com with ESMTP id <20061024124211.UZKS18511.mxfep03.bredband.com@ironport.bredband.com> for ; Tue, 24 Oct 2006 14:42:11 +0200 Received: from c-e8df71d5.029-19-73746f13.cust.bredbandsbolaget.se (HELO localhost.localdomain) ([213.113.223.232]) by ironport.bredband.com with ESMTP; 24 Oct 2006 14:42:11 +0200 From: Johan Rydberg To: The development of GRUB 2 References: <453DBE9A.7040300@intel.com> Date: Tue, 24 Oct 2006 15:11:19 +0200 In-Reply-To: <453DBE9A.7040300@intel.com> (mao bibo's message of "Tue, 24 Oct 2006 15:19:54 +0800") Message-ID: <87fydd3ll4.fsf@night.trouble.net> User-Agent: Gnus/5.110004 (No Gnus v0.4) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Subject: Re: [PATCH 3/3] grub EFI disk device enumberating 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, 24 Oct 2006 12:42:18 -0000 --=-=-= Content-Transfer-Encoding: quoted-printable "bibo,mao" writes: > Previously I posted this patch, now I repost again. > Any suggestion is welcome. I will just give a comment here. Please do not treat it as a full review, I'll leave that to Okuji. > +static int compare_ancestor_path(const grub_efi_device_path_t *parent, > + const grub_efi_device_path_t *dp2) > +{ > [...] > + while (1) > + { > + grub_efi_uint8_t type1, type2; > + grub_efi_uint8_t subtype1, subtype2; > + grub_efi_uint16_t len1, len2; > + int ret; > + > + if (GRUB_EFI_END_ENTIRE_DEVICE_PATH(parent)) > + break; > + > + type1 =3D GRUB_EFI_DEVICE_PATH_TYPE (parent); > + type2 =3D GRUB_EFI_DEVICE_PATH_TYPE (dp2); > + > + if (type1 !=3D type2) > + return (int) type2 - (int) type1; > + > + subtype1 =3D GRUB_EFI_DEVICE_PATH_SUBTYPE (parent); > + subtype2 =3D GRUB_EFI_DEVICE_PATH_SUBTYPE (dp2); > + > + if (subtype1 !=3D subtype2) > + return (int) subtype1 - (int) subtype2; > + > + len1 =3D GRUB_EFI_DEVICE_PATH_LENGTH (parent); > + len2 =3D GRUB_EFI_DEVICE_PATH_LENGTH (dp2); > + > + if (len1 !=3D len2) > + return (int) len1 - (int) len2; > + > + ret =3D grub_memcmp (parent, dp2, len1); > + if (ret !=3D 0) > + return ret; > + > + parent =3D (grub_efi_device_path_t *) ((char *) parent + len1); > + dp2 =3D (grub_efi_device_path_t *) ((char *) dp2 + len2); This code can be shorter; you only have to compare the lengths. If they match, you can do a memcmp on the whole device path.=20=20 Also, can you guarantee that the child path "dp2" is longer than the parent path? What it does is that it checks if "parent" is a subpath of "dp2", so a function name reflecting that probably fits better. Name functions after what they do, not what they are used to. Better argument names are also welcome. ~j --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2.2 (GNU/Linux) iD8DBQBFPhD63CqIy3K3X2ERAmXJAKCZsIREb/vY5Ax4bHCr1sEwIlh4PwCg0+Ho iiDoNn/pJPCc5Fk5kl17/Is= =BSOF -----END PGP SIGNATURE----- --=-=-=--