On Saturday 10 December 2005 11:05 am, Vesa Jääskeläinen 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 = function_that_could_fail ();
> /* do some stuff here. */
> if (rc)
> return rc;
Please do not change it in this way. The error subsystem of GRUB is based on
the idea of exceptions, that is to say, passing an error to higher levels
until it is caught explicitly. This is similar to C++, Java, Python or Ruby.
Unfortunately, C does not support exceptions directly, so we must emulate it.
So, what we must do is not to ignore an error where it happens.
I use Python as an example. Suppose this code:
def foo():
bar()
def bar()
baz()
print 'bar'
def baz()
raise RuntimeError, 'baz'
foo()
When executing this, Python does not print 'bar', because the exception raised
by baz is not caught by bar, and the control is immediately transferred to
foo.
In C, to emulate this behavior, we must do like this:
void
foo (void)
{
bar ();
}
void
bar (void)
{
baz ();
if (grub_errno != GRUB_ERR_NONE)
return;
grub_printf ("bar\n");
}
void
baz (void)
{
grub_error (GRUB_ERR_SOMETHING, "baz");
}
In other words, if you ignore an error set by calling a function and pass the
control to another, it violates the semantics. So, strictly speaking, you
must choose either of these when an error is set:
- deciding to ignore the error explicitly by setting GRUB_ERRNO to
GRUB_ERR_NONE
- handling the error by returning to the higher level or dealing with the
error locally (e.g. printing the error)
We (at least I) sometimes violate this rule by intention when knowing that it
is safe to ignore an error, simply because it is too heavy to deal with
errors all the time.
In your situation, things are a bit complicated. In my opinion, all you should
do is to save error information in a temporary place and reset it afterwards.
For example, you can define a function like this:
void
grub_error_push (void)
{
/* Push the current error in a stack and clear GRUB_ERRNO. */
...
}
void
grub_error_pop (void)
{
/* Pop the previous error and set GRUB_ERRNO. */
...
}
I don't know if this should be stack-based or flat. For example, you can
allocate space in a function and pass the pointer to a function which saves
current error information. In this case, functions should be named
grub_error_save and grub_error_restore.
The difficulty is that it is not very safe to use a heap to allocate space to
store error information, because such a memory allocation can generate
another error (due to memory shortage). The possibility is low, but we must
still consider it.
One way is to allocate such space in advance. For example, if we use the
stack-based approach (personally I prefer this for consistency), instead of
allocating space dynamically, we can allocate it statically. That is, at
linking time or at initialization time. This way limits the maximum number of
slots arbitrarily, but it should be enough for the reality (e.g. 10).
What do you think?
Okuji
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
