Re: problem in usage of grub_errno...

[Vesa Jääskeläinen]

I made a test implementation of the proposal. I also made some tests
with video subsystem and it eliminated problem I was having.

In case there is a similar case that I had, so it is necessary to reset
error state but not to discard the actual error message here is some
idea how it should be used:

---
/* Save possible old error message.  */
grub_error_push ();

/* Do your stuff here.  */
call_possibly_failing_function ();

if (grub_errno != GRUB_ERR_NONE)
  {
    /* Inform rest of the code that there is error (grub_errno
       is set). */
    return;
  }

/* Restore old error state by popping previous item from stack. */
grub_error_pop ();
---

I made a change to proposed error stack overflow indication mechanism as
there was possibility that assert message could get lost. Instead now it
marks an assert flag that overflow has happened. When error stack is
full and push is attempted, error message will be discarded and
grub_errno will be set to GRUB_ERR_NONE in order to other parts of the
system to work as excepted.

When prompt is about to be drawn again, I modified it to print whole
error stack in reverse order. I concluded that closest message to prompt
will be easiest to pick up. But the order can be changed if it is a problem.

I will wait couple of days for comments, if patch is not "killed" I will
commit it to CVS.

Thanks,
Vesa Jääskeläinen

Index: ChangeLog
===================================================================
RCS file: /sources/grub/grub2/ChangeLog,v
retrieving revision 1.210
diff -u -r1.210 ChangeLog
--- ChangeLog   10 Dec 2005 05:24:58 -0000      1.210
+++ ChangeLog   17 Dec 2005 17:32:13 -0000
@@ -1,3 +1,18 @@
+2005-12-17  Vesa Jaaskelainen  <address@hidden>
+
+       * kern/err.c (grub_error_push): Added new function to support error
+       stacks.
+       (grub_error_pop): Likewise.
+       (grub_error_stack_items): New local variable to support error stacks.
+       (grub_error_stack_pos): Likewise.
+       (grub_error_stack_assert): Likewise.
+       (GRUB_ERROR_STACK_SIZE): Added new define to configure maximum error
+       stack depth.
+       (grub_print_error): Added support to print errors from error stack.
+
+       * include/grub/err.h (grub_error_push): Added function prototype.
+       (grub_error_pop): Likewise.
+
 2005-12-09  Hollis Blanchard  <address@hidden>
 
        * configure.ac: Accept `powerpc64' as host_cpu.
Index: include/grub/err.h
===================================================================
RCS file: /sources/grub/grub2/include/grub/err.h,v
retrieving revision 1.10
diff -u -r1.10 err.h
--- include/grub/err.h  6 Nov 2005 22:19:59 -0000       1.10
+++ include/grub/err.h  17 Dec 2005 17:32:14 -0000
@@ -1,7 +1,7 @@
 /* err.h - error numbers and prototypes */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2004  Free Software Foundation, Inc.
+ *  Copyright (C) 2002-2005  Free Software Foundation, Inc.
  *
  *  GRUB is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -60,6 +60,8 @@
 
 grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...);
 void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_error_push) (void);
+int EXPORT_FUNC(grub_error_pop) (void);
 void EXPORT_FUNC(grub_print_error) (void);
 
 #endif /* ! GRUB_ERR_HEADER */
Index: kern/err.c
===================================================================
RCS file: /sources/grub/grub2/kern/err.c,v
retrieving revision 1.3
diff -u -r1.3 err.c
--- kern/err.c  4 Apr 2004 13:46:01 -0000       1.3
+++ kern/err.c  17 Dec 2005 17:32:14 -0000
@@ -1,7 +1,7 @@
 /* err.c - error handling routines */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2005  Free Software Foundation, Inc.
  *
  *  GRUB is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -22,11 +22,21 @@
 #include <grub/misc.h>
 #include <stdarg.h>
 
-#define GRUB_MAX_ERRMSG        256
+#define GRUB_MAX_ERRMSG                256
+#define GRUB_ERROR_STACK_SIZE  10
 
 grub_err_t grub_errno;
 char grub_errmsg[GRUB_MAX_ERRMSG];
 
+static struct
+{
+  grub_err_t errno;
+  char errmsg[GRUB_MAX_ERRMSG];
+} grub_error_stack_items[GRUB_ERROR_STACK_SIZE];
+
+static int grub_error_stack_pos;
+static int grub_error_stack_assert;
+
 grub_err_t
 grub_error (grub_err_t n, const char *fmt, ...)
 {
@@ -54,8 +64,72 @@
 }
 
 void
+grub_error_push (void)
+{
+  /* Only add items to stack, if there is enough room.  */
+  if (grub_error_stack_pos < GRUB_ERROR_STACK_SIZE)
+    {
+      /* Copy active error message to stack.  */
+      grub_error_stack_items[grub_error_stack_pos].errno = grub_errno;
+      grub_memcpy (grub_error_stack_items[grub_error_stack_pos].errmsg,
+                   grub_errmsg,
+                   sizeof (grub_errmsg));
+        
+      /* Advance to next error stack position.  */
+      grub_error_stack_pos++;
+    }
+  else
+    {
+      /* There is no room for new error message. Discard new error message
+         and mark error stack assertion flag.  */
+      grub_error_stack_assert = 1;
+    }
+
+  /* Allow further operation of other components by resetting
+     active errno to GRUB_ERR_NONE.  */
+  grub_errno = GRUB_ERR_NONE;
+}
+
+int
+grub_error_pop (void)
+{
+  if (grub_error_stack_pos > 0)
+    {
+      /* Pop error message from error stack to current active error.  */
+      grub_error_stack_pos--;
+      
+      grub_errno = grub_error_stack_items[grub_error_stack_pos].errno;
+      grub_memcpy (grub_errmsg,
+                   grub_error_stack_items[grub_error_stack_pos].errmsg,
+                   sizeof (grub_errmsg));
+                  
+      return 1;
+    }
+  else
+    {
+      /* There is no more items on error stack, reset to no error state.  */
+      grub_errno = GRUB_ERR_NONE;
+      
+      return 0;
+    }
+}
+
+void
 grub_print_error (void)
 {
-  if (grub_errno != GRUB_ERR_NONE)
-    grub_printf ("error: %s\n", grub_errmsg);
+  /* Print error messages in reverse order. First print active error message
+     and then empty error stack.  */
+  do
+    {
+      if (grub_errno != GRUB_ERR_NONE)
+        grub_printf ("error: %s\n", grub_errmsg);
+    } 
+  while (grub_error_pop ());
+  
+  /* If there was an assert while using error stack, report about it.  */
+  if (grub_error_stack_assert)
+    {
+      grub_printf ("assert: error stack overflow detected!\n");
+      grub_error_stack_assert = 0;
+    }
 }