fix usability problem in FreeBSD loader
PATCH 1/1 fix usability problem in FreeBSD loader
There's a minor usability problem with FreeBSD loader. E.g. if user runs freebsd_module first, the error message is confusing.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
2009-08-01 Robert Millan <[email protected]>
* loader/i386/bsd.c (grub_cmd_freebsd_loadenv, grub_cmd_freebsd_module) (grub_cmd_freebsd_module_elf): Abort with "You need to load the kernel first." when `kernel_type' is set to KERNEL_TYPE_NONE.
i386/bsd.c +13 -1
@@ -47,7 +47,7 @@#define MOD_BUF_ALLOC_UNIT 4096 -static int kernel_type; +static int kernel_type;/* 0 == KERNEL_TYPE_NONE */ static grub_dl_t my_mod;static grub_addr_t entry, entry_hi, kern_start, kern_end;static grub_uint32_t bootflags;
@@ -914,6 +914,10 @@ grub_cmd_freebsd_loadenv (grub_command_tchar *buf =0 , *curr, *next;int len; +if (kernel_type ==KERNEL_TYPE_NONE ) +return grub_error (GRUB_ERR_BAD_ARGUMENT , +"You need to load the kernel first." ); +if (kernel_type !=KERNEL_TYPE_FREEBSD )return grub_error (GRUB_ERR_BAD_ARGUMENT ,"only freebsd support environment" );
@@ -1004,6 +1008,10 @@ grub_cmd_freebsd_module (grub_command_tchar **modargv;char *type; +if (kernel_type ==KERNEL_TYPE_NONE ) +return grub_error (GRUB_ERR_BAD_ARGUMENT , +"You need to load the kernel first." ); +if (kernel_type !=KERNEL_TYPE_FREEBSD )return grub_error (GRUB_ERR_BAD_ARGUMENT ,"only freebsd support module" );
@@ -1066,6 +1074,10 @@ grub_cmd_freebsd_module_elf (grub_commangrub_file_t file =0 ;grub_err_t err; +if (kernel_type ==KERNEL_TYPE_NONE ) +return grub_error (GRUB_ERR_BAD_ARGUMENT , +"You need to load the kernel first." ); +if (kernel_type !=KERNEL_TYPE_FREEBSD )return grub_error (GRUB_ERR_BAD_ARGUMENT ,"only freebsd support module" );
Quoted 4 lines · Robert Millan
There's a minor usability problem with FreeBSD loader. E.g. if user runs
freebsd_modulefirst, the error message is confusing.
-static int kernel_type;
+static int kernel_type; /* 0 == KERNEL_TYPE_NONE */ I would prefer either
static int kernel_type = KERNEL_TYPE_NONE;
And it would be better to use an enum here rather than defines.
+ "You need to load the kernel first.");
+
if (kernel_type != KERNEL_TYPE_FREEBSD)
return grub_error (GRUB_ERR_BAD_ARGUMENT,
"only freebsd support module");
It seems that we have a bitrot in error message styles. We should agree on one style and use it everywhere
Other than these two remarks patch is good.
Quoted 13 lines
--
Robert MillanThe DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 4 lines
Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
Quoted 9 lines · Vladimir 'phcoder' Serbinenko
Quoted 4 lines · Robert Millan
There's a minor usability problem with FreeBSD loader. E.g. if user runs
freebsd_modulefirst, the error message is confusing.i386/bsd.c +1
-1-static int kernel_type; +static int kernel_type;/* 0 == KERNEL_TYPE_NONE */ I would prefer either
static intkernel_type=KERNEL_TYPE_NONE;
Ok.
Quoted 1 line
And it would be better to use an enum here rather than defines.
I'll fix that too.
Quoted 7 lines
i386/bsd.c +2
-0+ "You need to load the kernel first."); +if (kernel_type !=KERNEL_TYPE_FREEBSD )return grub_error (GRUB_ERR_BAD_ARGUMENT ,"only freebsd support module" );It seems that we have a bitrot in error message styles. We should agree on one style and use it everywhere
This error message, in particular, is copied from linux.c. I think we should capitalize the first letter of a phrase every time. I'll fix the other messages in this file too.
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."
Committed.
Quoted 42 lines · Robert Millan
Quoted 9 lines · Vladimir 'phcoder' Serbinenko
Quoted 4 lines · Robert Millan
There's a minor usability problem with FreeBSD loader. E.g. if user runs
freebsd_modulefirst, the error message is confusing.i386/bsd.c +1
-1-static int kernel_type; +static int kernel_type;/* 0 == KERNEL_TYPE_NONE */ I would prefer either
static intkernel_type=KERNEL_TYPE_NONE;Ok.
Quoted 1 line
And it would be better to use an enum here rather than defines.
I'll fix that too.
Quoted 7 lines
i386/bsd.c +2
-0+ "You need to load the kernel first."); +if (kernel_type !=KERNEL_TYPE_FREEBSD )return grub_error (GRUB_ERR_BAD_ARGUMENT ,"only freebsd support module" );It seems that we have a bitrot in error message styles. We should agree on one style and use it everywhere
This error message, in particular, is copied from
linux.c. I think we should capitalize the first letter of a phrase every time. I'll fix the other messages in this file too.Signature · 11 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." _______________________________________________ Grub-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 5 lines
Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all."