From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1KiXmR-0007sx-Lp for mharc-grub-devel@gnu.org; Wed, 24 Sep 2008 13:03:59 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KiXmN-0007rW-HS for grub-devel@gnu.org; Wed, 24 Sep 2008 13:03:55 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KiXmM-0007qz-AG for grub-devel@gnu.org; Wed, 24 Sep 2008 13:03:54 -0400 Received: from [199.232.76.173] (port=52772 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KiXmM-0007qt-2Z for grub-devel@gnu.org; Wed, 24 Sep 2008 13:03:54 -0400 Received: from ug-out-1314.google.com ([66.249.92.172]:60584) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KiXmL-0007lX-Hq for grub-devel@gnu.org; Wed, 24 Sep 2008 13:03:53 -0400 Received: by ug-out-1314.google.com with SMTP id z36so1793uge.17 for ; Wed, 24 Sep 2008 10:03:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:in-reply-to :references:content-type:date:message-id:mime-version:x-mailer; bh=jlj+iN9wsr1TNEffm36h0xoooOFYsiXJM0DxjElda2Y=; b=Ka6uKZdBl9GsZ+mj3Myd8m0fsoheoeZVrIKopZdVPMLnQYutiHIM5K2y8Gvvu5ms8a JZoXbd+tHhmEMVIKTgSlP3LsoWVgnc8SXqqpw5Sx8y0lCjQobHddouU0quxuQPirKjPf itUjak4z9EuwsESXmp94YCyJ5Wh1LmLOQBEx0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:in-reply-to:references:content-type:date:message-id :mime-version:x-mailer; b=DQoIDa175Q72TOMzHVdDPRwHi1Ru9k2lvIDhCrP3OEzFBsNQC9KX//Afxvkc8/xnoD Uu6h9osoXF0pi0HF5zQBx78Ax/S/xGaO20TxbH7KUygm62xJM8yQ/d9KG+egGQ1HY4xi IvTv1kS8Zf1Euly7+W5vNE17fvPEUWQYg4E64= Received: by 10.103.246.17 with SMTP id y17mr5016387mur.55.1222275828021; Wed, 24 Sep 2008 10:03:48 -0700 (PDT) Received: from ?192.168.1.100? (213.37.137.93.dyn.user.ono.com [213.37.137.93]) by mx.google.com with ESMTPS id e10sm4410578muf.14.2008.09.24.10.03.43 (version=SSLv3 cipher=RC4-MD5); Wed, 24 Sep 2008 10:03:45 -0700 (PDT) From: Javier =?ISO-8859-1?Q?Mart=EDn?= To: The development of GRUB 2 In-Reply-To: <1220131705.2622.19.camel@localhost> References: <20080704000829.GE4074@thorin> <1215135163.26019.44.camel@localhost> <20080704142125.GC2663@thorin> <1215182702.26019.130.camel@localhost> <20080704185723.GB32625@thorin> <1215204095.26019.142.camel@localhost> <20080705120757.GA1647@thorin> <1215282973.26019.183.camel@localhost> <20080719142707.GA23778@thorin> <20080830111753.GA16775@thorin> <1220131705.2622.19.camel@localhost> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-tomnYZDbEYKNUPFoYuAA" Date: Wed, 24 Sep 2008 19:05:33 +0200 Message-Id: <1222275933.18502.2.camel@localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: grub-probe detects ext4 wronly as ext2 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: Wed, 24 Sep 2008 17:03:55 -0000 --=-tomnYZDbEYKNUPFoYuAA Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Well, seeing that Robert is back (and what a torrent of activity!), I'll shamelessly try to resubmit my last post from nearly a month ago in the hope that he'll notice it. El s=C3=A1b, 30-08-2008 a las 23:28 +0200, Javier Mart=C3=ADn escribi=C3=B3= : > Hello there! It's nice to be in town again, though post-vacation > syndrome is hitting me full... Resuming the thread, >=20 > El s=C3=A1b, 30-08-2008 a las 13:17 +0200, Robert Millan escribi=C3=B3: > > On Mon, Aug 11, 2008 at 04:14:00PM +0200, Javier Mart=C3=ADn wrote: > > > >> > This overrides the grub_errno and grub_errmsg provided by grub_d= isk_read and > > > >> > replaces them with values that hide the true problem. If there = was a disk > > > >> > read error, we really want to know about it from the lower layer= . > > > >> Well, the old version did just the same (even worse, because the m= essage > > > >> was generic). What would be the correct path of action here? I mea= n, how > > > >> can we propagate the error messages? > > > > > > > > It shouldn't call grub_error(). > > >=20 > > > So in the cases the fail is caused by an underlying error (like I/O) > > > the code should just return failure and leave the old error in errno > > > and errmsg? > >=20 > > Yes. > Ok, so that's already done in the previous patch. >=20 > >=20 > > > static struct grub_ext2_data * > > > grub_ext2_mount (grub_disk_t disk) > > > { > > > struct grub_ext2_data *data; > > > + const char *local_error =3D 0; > >=20 > > Please use NULL for pointers. > I don't object at all, but 0 is used throughout the GRUB code to > represent null pointers (see the args struct in any command source > file), so I don't understand the criterion being applied here. >=20 > >=20 > > > - if (grub_errno) > > > + if (grub_errno !=3D GRUB_ERR_NONE) > >=20 > > Why? > 1st, because the condition being checked is explicit and thus clearer. > These int->bool C conventions are, even though enshrined by ANSI and > thus pretty standard and reliable, irky at best and due only to the lack > of a proper boolean type. As I think I stated before, the only cases I > use such "cast" is when checking for null pointers and when using > variables that are explicitly boolean in nature. > 2nd, because the new code does not assume that GRUB_ERR_NONE is defined > to zero. Even though this definition will most likely never change, we > might one day decide that -42 is a better value for success. > 3rd, because in addition to all that, with any sensible compiler, the > additional binary size cost is nothing at all. >=20 > >=20 > > > - goto fail; > > > + EXT2_DRIVER_MOUNT_FAIL(0); > >=20 > > I find very little use in this. I assume it's supposed to simplify thi= ngs, > > but in fact it adds an extra level of indirection for someone who's rea= ding > > the code. It provides no runtime improvement, and it's inconsistent wi= th what > > we do elsewhere. > It is not meant to provide any runtime improvement, it's just for > consistency: since local_error is already zero, a "goto fail" would > suffice but I think this uniformity adds readability, not hinders it: > the referenced macro is at the very top of the function, not on a > included header, so the reference is not very time-consuming; and it's > not very complex, so it only needs to be read once. Besides, most > compilers should just optimize the extra assignment away given that the > value of local_error is ct-known for the code paths leading to that > statement and it is not a "volatile" variable. >=20 > >=20 > > > + /* Only call grub_error if the fail was _not_ caused by underlying= errors. */ > >=20 > > No need to document this. It's the same deal in a huge amount of routi= nes > > throurough the GRUB source. > OK, removed the comment. Maybe a similar comment will be fine over the > macro, though. >=20 > That was all, folks! Given that I (think I) addressed your two main > objections, I won't send a new patch right now. I will continue to read > the list this month, but my availability is likely to vary wildly, as I > will be trapped by the ensnaring bureaucracy of the Spanish > universities, scholarships, etc. >=20 > -Habbit --=-tomnYZDbEYKNUPFoYuAA Content-Type: application/pgp-signature; name=signature.asc Content-Description: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada digitalmente -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iQIVAwUASNpzXaSl+Fbdeo72AQLGqg//bQP0HgITWDbxauJcYw7kgwMdtTQNIYYe FVIJXuyrOz1e9bLYtm3Y3ZR8NEv1iPU6TCBcnt5mZQaqJND/7wt1HajuccxWIDLN EvVTL5atspigUCSxuihn0FruxOLVywx3VF886oehkstR2e8UANgGDDasddGYBagQ SQ/lAZOxqNu1ar/fRO8XdMscNTSmE18Mjt7OgT0gSs7AdC6hh70vRQ1lQKL/sXN8 2GmWCHa44ifJYJ2F29YZR7FCzhC1lBNouAt4HpGc+/0rYIIbDOS63+5P0az4iVdF NenXpEHJumbi1Qkr3B80o4RiPnuCQ008NX1MefrUr8VV6TLcC7vT1nrjzIwCHfck Hq+lUovIcFnI7AOHf9BIxuCQ4dR1+CuELmwM5hRBZZovhCrZMdJW6ozVLOUW/Jrk PI8MfjfVTxEfwoTmbp6YEOFTeyD89a5HpYnOaJWNl+TWXSBFPac8IZg/301QKxgH M4XHT8xpIOqwYOEGykzL6zcfietBrFnA8dmQAV3pf6EasoBCSWBJUH2k1eb9BgoV wPs7u1qSkWbPg+h4Z/5S8raST4HDJfJ9LEPk4HFHmlyVdDMG6VUm5kG9foqxsbe5 cyv6ulJJ0EPNuO4XT8g4xpPfHrjvjPFv0IDq/v9p5AtVAuXPAxaEB6aZ4QwQpd/3 7znWIOGM8bA= =2H1u -----END PGP SIGNATURE----- --=-tomnYZDbEYKNUPFoYuAA--