Skip to contentSkip to conversation

Reset grub_errno after embedding test on non-fatal failures

PATCH In review Mario Limonciello 3 participants 6 messages Sep 26, 2011 – Nov 8, 2011

lore
Share & tools
lore

Mario Limonciello

author
#0 lore raw

PATCH 1/1 Reset grub_errno after embedding test on non-fatal failures

Hello,

In additional MinGW testing, i've found that there are instances where the embedding test used in grub-setup will fail to seek on certain devices (such as card readers where no cards are physically present). This sets grub_errno during the embedding test as the different disks in the system are tested. It gets set even when installing to a different disk (all the physical disks are seeked). Consequently grub_errno remains set up to the point when the core image is to be written out and mistakingly will indicate grub-setup failed for a seek error that happened at the start.

So this patch resets grub_errno while completing the embedding test only on the non fatal failures.

Thanks,

Signature · 3 lines
*Mario Limonciello*
Linux Engineer
*Dell*| OS Engineering

=== modified file 'ChangeLog'

ChangeLog +4 -0
@@ -1,3 +1,7 @@
+2011-09-26  Mario Limonciello <[email protected]>
+
+	* Reset grub_errno after embedding test on a non-fatal failure.
+
 2011-09-17  Grégoire Sutre  <[email protected]>
 
 	* Makefile.util.def (grub-mkrelpath): Add LIBUTIL for getrawpartition(3)

=== modified file 'grub-core/partmap/msdos.c'

partmap/msdos.c +1 -1
@@ -253,7 +253,7 @@
 	return grub_errno;
       for (i = 0; i < *nsectors; i++)
 	(*sectors)[i] = 1 + i;
-      return GRUB_ERR_NONE;
+      return grub_error(GRUB_ERR_NONE,"");
     }
 
   if (end <= 1)
#1 lore raw

partmap/msdos.c line 256

Quoted 2 lines · Mario Limonciello
partmap/msdos.c +1 -1
-      return GRUB_ERR_NONE;
+      return grub_error(GRUB_ERR_NONE,"");

This is obviously the wrong place. It's the successful return clause. if grub_errno isn't 0 at this point then the code is wrong somewhere else.

Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
#2 lore raw

Hi,

I finally got fed up with having no 'c' to continue when pager was set to
1. This may be a bit hackish, so I'm bracing for the flame ;), but it's here if it'll help someone else.

--------------

normal/term.c 2011-09-27 19:33:34 +0000 renamed from normal/term.c 2011-04-10 11:56:23 +0000 +13 -7
@@ -40,7 +40,7 @@
 static struct term_state *term_states = NULL;
 
 /* If the more pager is active.  */
-static int grub_more;
+int grub_more, real_grub_more;
 
 static void
 putcode_real (grub_uint32_t code, struct grub_term_output *term);

@@ -60,11 +60,11 @@
   grub_uint16_t *pos;
   grub_term_output_t term;
   grub_uint32_t *unicode_str, *unicode_last_position;
+  const char *PROMPT_STRING = "--MORE-- ('c' to disable paging for the remainder of this command)";
 
   pos = grub_term_save_pos ();
 
-  grub_utf8_to_ucs4_alloc ("--MORE--", &unicode_str,
-                          &unicode_last_position);
+  grub_utf8_to_ucs4_alloc (PROMPT_STRING, &unicode_str, &unicode_last_position);
 
   if (!unicode_str)
     {

@@ -87,7 +87,7 @@
   /* Remove the message.  */
   grub_term_restore_pos (pos);
   FOR_ACTIVE_TERM_OUTPUTS(term)
-    grub_print_spaces (term, 8);
+    grub_print_spaces (term, grub_strlen(PROMPT_STRING));
   grub_term_restore_pos (pos);
   grub_free (pos);
 

@@ -99,6 +99,11 @@
       for (state = term_states; state; state = state->next)
        state->num_lines--;
     }
+  else if (key == 'c' || key == 'C')
+    {
+      grub_more = 0;
+      grub_normal_reset_more ();
+    }
   else
     grub_normal_reset_more ();
 }

@@ -107,9 +112,10 @@
 grub_set_more (int onoff)
 {
   if (onoff == 1)
-    grub_more++;
-  else
-    grub_more--;
+    real_grub_more++;
+  else if (real_grub_more > 0)
+    real_grub_more--;
+  grub_more = real_grub_more;
   grub_normal_reset_more ();
 }

=== modified file 'grub-core/script/execute.c'

script/execute.c 2011-09-27 19:25:12 +0000 renamed from script/execute.c 2010-12-02 09:31:06 +0000 +2 -0
@@ -445,11 +445,13 @@
 {
   int ret;
   char errnobuf[ERRNO_DIGITS_MAX + 1];
+  extern int grub_more, real_grub_more;
 
   if (cmd == 0)
     return 0;
 
   ret = cmd->exec (cmd);
+  grub_more = real_grub_more;
 
   grub_snprintf (errnobuf, sizeof (errnobuf), "%d", ret);
   grub_env_set ("?", errnobuf);
#3 lore raw
Quoted 5 lines · Seth Goldberg

Hi,

I finally got fed up with having no 'c' to continue when pager was
set to 1. This may be a bit hackish, so I'm bracing for the flame ;), but it's here if it'll help someone else.

It's a good idea but I feel like the point where we restore original setting is badly chosen. Consider:
for x in *; do
echo $x;
done
You probably want to skip until the end of the script. I think it's reasonable to restore the setting when user interaction is required. This will have a neat effect of constraining the changes to term subsystem. Since grub_getkey/grub_checkkey are in core changes to them should be minimalised. They could just keep a counter how many times they were called.

Quoted 90 lines

--------------

normal/term.c 2011-09-27 19:33:34 +0000 renamed from normal/term.c 2011-04-10 11:56:23 +0000 +3 -3
@@ -40,7 +40,7 @@
 static struct term_state *term_states = NULL;
 
 /* If the more pager is active.  */
-static int grub_more;
+int grub_more, real_grub_more;
 
 static void
 putcode_real (grub_uint32_t code, struct grub_term_output *term);

@@ -60,11 +60,11 @@
   grub_uint16_t *pos;
   grub_term_output_t term;
   grub_uint32_t *unicode_str, *unicode_last_position;
+  const char *PROMPT_STRING = "--MORE-- ('c' to disable paging for
 the remainder of this command)";
 
   pos = grub_term_save_pos ();
 
-  grub_utf8_to_ucs4_alloc ("--MORE--", &unicode_str,
-                          &unicode_last_position);
+  grub_utf8_to_ucs4_alloc (PROMPT_STRING, &unicode_str,
 &unicode_last_position);
 

   if (!unicode_str)
     {
normal/term.c 2011-09-27 19:33:34 +0000 renamed from normal/term.c 2011-04-10 11:56:23 +0000 +10 -4
@@ -87,7 +87,7 @@
   /* Remove the message.  */
   grub_term_restore_pos (pos);
   FOR_ACTIVE_TERM_OUTPUTS(term)
-    grub_print_spaces (term, 8);
+    grub_print_spaces (term, grub_strlen(PROMPT_STRING));
   grub_term_restore_pos (pos);
   grub_free (pos);
 

@@ -99,6 +99,11 @@
       for (state = term_states; state; state = state->next)
        state->num_lines--;
     }
+  else if (key == 'c' || key == 'C')
+    {
+      grub_more = 0;
+      grub_normal_reset_more ();
+    }
   else
     grub_normal_reset_more ();
 }

@@ -107,9 +112,10 @@
 grub_set_more (int onoff)
 {
   if (onoff == 1)
-    grub_more++;
-  else
-    grub_more--;
+    real_grub_more++;
+  else if (real_grub_more > 0)
+    real_grub_more--;
+  grub_more = real_grub_more;
   grub_normal_reset_more ();
 }

=== modified file 'grub-core/script/execute.c'

script/execute.c 2011-09-27 19:25:12 +0000 renamed from script/execute.c 2010-12-02 09:31:06 +0000 +2 -0
@@ -445,11 +445,13 @@
 {
   int ret;
   char errnobuf[ERRNO_DIGITS_MAX + 1];
+  extern int grub_more, real_grub_more;
 
   if (cmd == 0)
     return 0;
 
   ret = cmd->exec (cmd);
+  grub_more = real_grub_more;
 
   grub_snprintf (errnobuf, sizeof (errnobuf), "%d", ret);
   grub_env_set ("?", errnobuf);

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
#5 lore raw

Re: (Subject on previous email)

Quoted 4 lines · Seth Goldberg

Sorry about that -- the subject was incorrect on that last email I
sent :(.

Please avoid replying to random e-mail to make a new post, even when changing subject. It adds In-Reply-To header which wreaks havoc with some archives and mail clients

Quoted 7 lines

--S

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko