* [PATCH] Reset grub_errno after embedding test on non-fatal failures
@ 2011-09-26 20:27 Mario Limonciello
2011-09-26 20:32 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello @ 2011-09-26 20:27 UTC (permalink / raw)
To: grub-devel@gnu.org
[-- Attachment #1.1: Type: text/plain, Size: 776 bytes --]
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,
--
*Mario Limonciello*
Linux Engineer
*Dell*| OS Engineering
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: errno.patch --]
[-- Type: text/x-diff; name="errno.patch", Size: 779 bytes --]
=== modified file 'ChangeLog'
--- ChangeLog 2011-09-17 21:40:10 +0000
+++ ChangeLog 2011-09-26 20:21:14 +0000
@@ -1,3 +1,7 @@
+2011-09-26 Mario Limonciello <mario_limonciello@dell.com>
+
+ * Reset grub_errno after embedding test on a non-fatal failure.
+
2011-09-17 Grégoire Sutre <gregoire.sutre@gmail.com>
* Makefile.util.def (grub-mkrelpath): Add LIBUTIL for getrawpartition(3)
=== modified file 'grub-core/partmap/msdos.c'
--- grub-core/partmap/msdos.c 2011-07-07 21:52:58 +0000
+++ grub-core/partmap/msdos.c 2011-09-26 20:22:52 +0000
@@ -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)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Reset grub_errno after embedding test on non-fatal failures
2011-09-26 20:27 [PATCH] Reset grub_errno after embedding test on non-fatal failures Mario Limonciello
@ 2011-09-26 20:32 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-09-27 19:38 ` Seth Goldberg
2011-09-27 19:39 ` (Subject on previous email) Seth Goldberg
0 siblings, 2 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-09-26 20:32 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 333 bytes --]
On 26.09.2011 22:27, Mario Limonciello wrote:
> - 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.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Reset grub_errno after embedding test on non-fatal failures
2011-09-26 20:32 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2011-09-27 19:38 ` Seth Goldberg
2011-11-08 14:36 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-09-27 19:39 ` (Subject on previous email) Seth Goldberg
1 sibling, 1 reply; 6+ messages in thread
From: Seth Goldberg @ 2011-09-27 19:38 UTC (permalink / raw)
To: The development of GNU GRUB
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.
--------------
--- grub-core/normal/term.c 2011-04-10 11:56:23 +0000
+++ grub-core/normal/term.c 2011-09-27 19:33:34 +0000
@@ -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'
--- grub-core/script/execute.c 2010-12-02 09:31:06 +0000
+++ grub-core/script/execute.c 2011-09-27 19:25:12 +0000
@@ -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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* (Subject on previous email)
2011-09-26 20:32 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-09-27 19:38 ` Seth Goldberg
@ 2011-09-27 19:39 ` Seth Goldberg
2011-09-28 21:45 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 1 reply; 6+ messages in thread
From: Seth Goldberg @ 2011-09-27 19:39 UTC (permalink / raw)
To: The development of GNU GRUB
Sorry about that -- the subject was incorrect on that last email I sent :(.
--S
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: (Subject on previous email)
2011-09-27 19:39 ` (Subject on previous email) Seth Goldberg
@ 2011-09-28 21:45 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-09-28 21:45 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 533 bytes --]
On 27.09.2011 21:39, Seth Goldberg wrote:
>
> 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
> --S
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Reset grub_errno after embedding test on non-fatal failures
2011-09-27 19:38 ` Seth Goldberg
@ 2011-11-08 14:36 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 0 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-11-08 14:36 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 3309 bytes --]
On 27.09.2011 21:38, Seth Goldberg wrote:
> 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.
>
>
> --------------
> --- grub-core/normal/term.c 2011-04-10 11:56:23 +0000
> +++ grub-core/normal/term.c 2011-09-27 19:33:34 +0000
> @@ -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'
> --- grub-core/script/execute.c 2010-12-02 09:31:06 +0000
> +++ grub-core/script/execute.c 2011-09-27 19:25:12 +0000
> @@ -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
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-08 14:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-26 20:27 [PATCH] Reset grub_errno after embedding test on non-fatal failures Mario Limonciello
2011-09-26 20:32 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-09-27 19:38 ` Seth Goldberg
2011-11-08 14:36 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-09-27 19:39 ` (Subject on previous email) Seth Goldberg
2011-09-28 21:45 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).