From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1ElBWH-0003gc-DT for mharc-grub-devel@gnu.org; Sat, 10 Dec 2005 15:40:37 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1ElBWG-0003gX-3F for grub-devel@gnu.org; Sat, 10 Dec 2005 15:40:36 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1ElBWF-0003gL-It for grub-devel@gnu.org; Sat, 10 Dec 2005 15:40:35 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ElBWF-0003gI-DE for grub-devel@gnu.org; Sat, 10 Dec 2005 15:40:35 -0500 Received: from [157.24.2.30] (helo=smtp1.cc.lut.fi) by monty-python.gnu.org with esmtp (Exim 4.34) id 1ElBXf-00042m-Kr for grub-devel@gnu.org; Sat, 10 Dec 2005 15:42:04 -0500 Received: from localhost (smtp1 [127.0.0.1]) by smtp1.cc.lut.fi (Postfix) with ESMTP id 784C970175 for ; Sat, 10 Dec 2005 22:40:07 +0200 (EET) Received: from smtp1.cc.lut.fi ([127.0.0.1]) by localhost (smtp1.cc.lut.fi [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 25563-03 for ; Sat, 10 Dec 2005 22:40:07 +0200 (EET) Received: from [192.168.1.100] (lk4-a-4-1.lnet.lut.fi [157.24.102.107]) by smtp1.cc.lut.fi (Postfix) with ESMTP id 61AB370098 for ; Sat, 10 Dec 2005 22:12:07 +0200 (EET) Message-ID: <439B369D.7020909@nic.fi> Date: Sat, 10 Dec 2005 22:12:13 +0200 From: =?ISO-8859-1?Q?Vesa_J=E4=E4skel=E4inen?= User-Agent: Thunderbird 1.4.1 (Windows/20051006) MIME-Version: 1.0 To: The development of GRUB 2 References: <4399FE01.7010402@nic.fi> <87mzj9q4z7.fsf@xs4all.nl> <439AA87B.9050607@nic.fi> <200512101411.11612.okuji@enbug.org> In-Reply-To: <200512101411.11612.okuji@enbug.org> X-Enigmail-Version: 0.93.0.0 Content-Type: text/plain; charset=ISO-8859-1 X-Virus-Scanned: by lut.fi Content-Transfer-Encoding: quoted-printable Subject: Re: problem in usage of grub_errno... 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: Sat, 10 Dec 2005 20:40:36 -0000 Yoshinori K. Okuji wrote: > On Saturday 10 December 2005 11:05 am, Vesa J=E4=E4skel=E4inen wrote: >> Changing coding guide of using grub_errno we could probably get this >> issue "working". But then there is still problem with lost error >> messages if subsequent code fails with real error. >> >> If we just zero out grub_errno in code before printing out error(s). >> Then the actual error message will get lost. So it is important not to >> zero out spuriously grub_errno. >> >> Good way of checking for errors could be like: >> -- >> grub_errno_t rc; >> >> rc =3D function_that_could_fail (); >> /* do some stuff here. */ >> if (rc) >> return rc; >=20 > Please do not change it in this way. The error subsystem of GRUB is bas= ed on=20 > the idea of exceptions, that is to say, passing an error to higher leve= ls=20 > until it is caught explicitly. This is similar to C++, Java, Python or = Ruby.=20 > Unfortunately, C does not support exceptions directly, so we must emula= te it.=20 > So, what we must do is not to ignore an error where it happens. [snip] > In other words, if you ignore an error set by calling a function and pa= ss the=20 > control to another, it violates the semantics. So, strictly speaking, y= ou=20 > must choose either of these when an error is set: >=20 > - deciding to ignore the error explicitly by setting GRUB_ERRNO to=20 > GRUB_ERR_NONE >=20 > - handling the error by returning to the higher level or dealing with t= he=20 > error locally (e.g. printing the error) >=20 > We (at least I) sometimes violate this rule by intention when knowing t= hat it=20 > is safe to ignore an error, simply because it is too heavy to deal with= =20 > errors all the time. Perhaps we could put this information somewhere on wiki? :) It could help a bit new developers and would also give some guidelines how error handling should be implemented. > In your situation, things are a bit complicated. In my opinion, all you= should=20 > do is to save error information in a temporary place and reset it after= wards.=20 > For example, you can define a function like this: >=20 > void > grub_error_push (void) > { > /* Push the current error in a stack and clear GRUB_ERRNO. */ > ... > } >=20 > void > grub_error_pop (void) > { > /* Pop the previous error and set GRUB_ERRNO. */ > ... > } >=20 > I don't know if this should be stack-based or flat. For example, you ca= n=20 > allocate space in a function and pass the pointer to a function which s= aves=20 > current error information. In this case, functions should be named=20 > grub_error_save and grub_error_restore. >=20 > The difficulty is that it is not very safe to use a heap to allocate sp= ace to=20 > store error information, because such a memory allocation can generate=20 > another error (due to memory shortage). The possibility is low, but we = must=20 > still consider it. >=20 > One way is to allocate such space in advance. For example, if we use th= e=20 > stack-based approach (personally I prefer this for consistency), instea= d of=20 > allocating space dynamically, we can allocate it statically. That is, a= t=20 > linking time or at initialization time. This way limits the maximum num= ber of=20 > slots arbitrarily, but it should be enough for the reality (e.g. 10). >=20 > What do you think? Sounds good. Enhancing the error handling could solve this problem. Stack based approach here sounds more reasonable. I think we should also modify grub_print_error () in way that it would print out all messages from stack too. Perhaps add some kind of boolean information to pop function to tell whether there are more entries in sta= ck. Something like this: -- do { if (grub_errno !=3D GRUB_ERR_NONE) grub_printf (...) } while (grub_error_pop ()); -- In case of stack is empty, grub_error_pop would set grub_errno to GRUB_ERR_NONE and return zero. If there is still entries left grub_errno and error string would be copied from stack and non-zero value would be returned. If the stack is about to be filled full, it could show some kind of assert message to user and keep stack at it's current state. One option could be that if there is room for one error message when push comes it would put assert message there and then refuse to push more messages. So new error messages would override latests ones. Most likely the first error message is the most important. Only thing here is what is preferred order to display error messages. It would be more traditional to display first error message first and then error message that came after that. Stack model for using it sounds reasonable, but for printing it, reverse order is more reasonable. Now if we think about the font manager issue. First it should search from it's cache for fonts and if it could not find it then do the following: 1. grub_error_push () 2. read glyph from disk 3. if there is error, ->leave & return dummy glyph (but do not release fo= nt) 4. if all went correctly, grub_error_pop () This would leave last error message to stack and then newest one from read glyph message is also shown to user. Assuming that there is understandable glyphs :). This would require that either fonts are validated at load time or then assumption that fonts will contain correct information. If there is only couple of displayable fonts corrupted then at least some of the message would be readable by the user. How this sounds to you ?