Hi, attached patches rework the device string parser in FILO so it should be easier to understand, and adds some more ways to specify devices/partitions. Specifically:
2-refactor-dev-parsing.diff: - copy_path_to_filo_bootline() erases the target location instead of expecting the callers to do so - callers of copy_path_to_filo_bootline() don't erase the target variables themselves. This fixes an issue where the function is called without the variable being erased. I couldn't think of any plausible scenario in which the old behaviour is desirable. 3-rewrite-drive-specification-parser.diff: - copy_path_to_filo_bootline is rewritten for clarity. - copy_path_to_filo_bootline now supports @address syntax in grub-style device names. - use strtoul instead of a custom number parser (theoretically allows for >99 disks/partitions, but I guess we'll never have to work with that) The old implementation of the function was a mess of hidden state dragged around in incidental configuration in variables. The current code flow should be easier to follow. 4-handle-partition-only-changes.diff: - Allow new syntax "(,N)" (grub-style) or "N:" (filo/linux-style) which simply switches to a different partition on the current device. Use case: one partition with filo.lst, another with kernels, filo.lst references the kernel as "5:/boot/kernel" and it works no matter the drive name. Signed-off-by: Patrick Georgi <[email protected]> -- Patrick Georgi SINA-Development - High Security secunet Security Networks AG - Mergenthalerallee 77 - 65760 Eschborn, Germany Phone +49 201 54 54-3610 - Fax +49 201 54 54-1325 - www.secunet.com Sitz: Kronprinzenstraße 30, 45128 Essen / Amtsgericht Essen HRB 13615 Vorstand: Dr. Rainer Baumgart (Vors.), Thomas Koelzer, Thomas Pleines Aufsichtsratsvorsitzender: Dr. Karsten Ottenberg
Index: filo/main/grub/builtins.c
===================================================================
--- filo.orig/main/grub/builtins.c
+++ filo/main/grub/builtins.c
@@ -304,7 +304,6 @@ static int configfile_func(char *arg, in
extern int is_opened, keep_cmdline_running;
/* Check if the file ARG is present. */
- temp_space[0]=0;
copy_path_to_filo_bootline(arg, temp_space, 1);
if (temp_space[0]==0) {
return help_func("configfile",0);
@@ -317,7 +316,6 @@ static int configfile_func(char *arg, in
file_close();
/* Copy ARG to CONFIG_FILE. */
- memset(config_file, 0, 128);
copy_path_to_filo_bootline(arg, config_file, 1);
/* Force to load the configuration file. */
@@ -666,6 +664,8 @@ void copy_path_to_filo_bootline(char *ar
int i, len;
+ path[0] = 0;
+
/* Clean up */
memset(devicename, 0, 16);
memset(drivername, 0, 16);
@@ -758,7 +758,6 @@ void copy_path_to_filo_bootline(char *ar
/* initrd */
static int initrd_func(char *arg, int flags)
{
- initrd_space[0]=0; // Erase string
copy_path_to_filo_bootline(arg, initrd_space, 1);
if (!file_open(initrd_space)) {
initrd_space[0]=0; // Erase string
@@ -897,11 +896,7 @@ static int kernel_func(char *arg, int fl
kernel_type = KERNEL_TYPE_NONE;
- /* clear out boot_line. Kernel is the first thing */
- boot_line[0] = 0; // Erase string
-
/* Get the real boot line and extract the kernel name */
- temp_space[0] = 0; // Erase string
copy_path_to_filo_bootline(arg, temp_space, 1);
i=0; while ((temp_space[i] != 0) && (temp_space[i]!=' ')) i++;
temp_space[i] = 0;
@@ -1339,7 +1334,6 @@ static int root_func(char *arg, int flag
{
int len;
- root_device[0] = 0; /* Clear root device */
copy_path_to_filo_bootline(arg, root_device, 0);
/* The following code handles an extra case
@@ -1810,7 +1804,6 @@ static int cat_func(char *arg, int flags
char buf[4096];
int len;
- temp_space[0]=0;
copy_path_to_filo_bootline(arg, temp_space, 1);
if (temp_space[0]==0) {
return help_func("cat",0);
Index: filo/main/grub/grub.c
===================================================================
--- filo.orig/main/grub/grub.c
+++ filo/main/grub/grub.c
@@ -159,7 +159,6 @@ int probe_menulst(char *bootdevice, char
strncat(menulst, filename, 256);
/* Set string to zero: */
- config_file[0] = 0;
copy_path_to_filo_bootline(menulst, config_file, use_root);
if (file_open(config_file)) {
/* We found a config file. Bail out */
Index: filo/main/grub/builtins.c
===================================================================
--- filo.orig/main/grub/builtins.c
+++ filo/main/grub/builtins.c
@@ -650,6 +650,30 @@ static struct builtin builtin_hiddenmenu
};
/**
+ * @param arg linux style driver specifier
+ * @param drivername driver name (out)
+ * @param disk disk number (out)
+ * @return length of parsed string
+ */
+static
+int parse_linux_style_driver(char *arg, char *drivername, int *disk)
+{
+ int i;
+ i = 0;
+ while ((i < 16) && (isalpha(arg[i]))) {
+ drivername[i] = arg[i];
+ i++;
+ }
+
+ if (i > 0) {
+ drivername[--i] = '\0';
+ *disk = arg[i]-'a';
+ i++;
+ }
+ return i;
+}
+
+/**
* @param arg source pointer with grub device names
* @param path destination pointer (will be filled with filo device names)
* @param use_rootdev values other than zero mean the root device set by the "root"
@@ -661,98 +685,87 @@ void copy_path_to_filo_bootline(char *ar
char devicename[16];
char drivername[16];
int disk, part;
+ unsigned long addr;
int i, len;
-
- path[0] = 0;
-
- /* Clean up */
memset(devicename, 0, 16);
memset(drivername, 0, 16);
+ disk = -1;
+ part = -1;
+ addr = -1;
- /* Copy over the driver name: "hd", "ud", "sd" ... */
if (arg[0] == '(') {
+ // grub style device specifier
i = 1;
/* Read until we encounter a number, a comma or a closing
* bracket
*/
- while ((i <= 16) && (arg[i]) && (!isdigit(arg[i])) && (arg[i] != ',')
- && (arg[i] != ')')) {
+ while ((i <= 16) && (isalpha(arg[i]))) {
drivername[i - 1] = arg[i];
i++;
}
- }
- disk = -1;
- part = -1;
+ if (isdigit(arg[i])) {
+ char *postnum;
+ disk = strtoul(arg+i, &postnum, 10);
+ i = postnum - arg;
+ }
- len = strlen(drivername);
- if (len) { /* We have a driver. No idea if it exists though */
- // The driver should decide this:
- len++; // skip driver name + opening bracket
-
- // XXX put @ handling in here, too for flash@addr and mem@addr
-
- if (isdigit(arg[len])) {
- disk = arg[len] - '0';
- len++;
- if (isdigit(arg[len])) { /* More than 9 drives? */
- /* ok, get one more number. No more than 99 drives */
- disk *= 10;
- disk += arg[len] - '0';
- len++;
- }
- }
- if (arg[len] == ',') {
- len++;
- part = arg[len] - '0';
- len++;
- if (isdigit(arg[len])) { /* More than 9 partitions? */
- /* ok, get one more number. No more than 99
- * partitions */
- part *= 10;
- part += arg[len] - '0';
- len++;
- }
- }
- if (arg[len] != ')') {
- grub_printf("Drive Error.\n");
- // set len = 0 --> just copy the drive name
- len = 0;
- } else {
- len++; // skip closing bracket
+ if (arg[i] == ',') {
+ char *postnum;
+ part = strtoul(arg+i+1, &postnum, 10) + 1;
+ i = postnum - arg;
}
- }
- if (disk == -1) {
- int cnt = 0;
- len = 0;
- while ((arg[cnt] != 0) && (arg[cnt+1] != 0)) {
- if (arg[cnt] == ':' && arg[cnt+1] == '/') {
- /* The user did specify a FILO name already */
- len = cnt;
- break;
- }
- cnt++;
+ if (arg[i] == '@') {
+ char *postnum;
+ addr = strtoul(arg+i+1, &postnum, 0);
+ i = postnum - arg;
}
- } else {
- if (part == -1) { // No partition
- sprintf(devicename, "%s%c:", drivername, disk + 'a');
- } else { // both disk and partition
- sprintf(devicename, "%s%c%d:", drivername, disk + 'a', part + 1);
+
+ if (arg[i] == ')') i++;
+
+ arg += i;
+ } else if ((use_rootdev == 0) || (strchr(arg, ':') != NULL)) {
+ // linux-style device specifier or
+ // leading device name required (assume it's linux-style then)
+ i = parse_linux_style_driver(arg, drivername, &disk);
+
+ if (isdigit(arg[i])) {
+ char *postnum;
+ part = strtoul(arg+i, &postnum, 10);
+ i = postnum - arg;
}
- strncat(path, devicename, BOOT_LINE_LENGTH);
- arg += len; // skip original drive name
- }
- if (use_rootdev && !len) { // No drive was explicitly specified
- if (strlen(root_device)) { // But someone set a root device
- strncat(path, root_device, BOOT_LINE_LENGTH);
+ if (arg[i] == '@') {
+ char *postnum;
+ addr = strtoul(arg+i+1, &postnum, 0);
+ i = postnum - arg;
}
+
+ if (arg[i] == ':') i++;
+ arg += i;
}
- /* Copy the rest over */
- strncat(path, arg, BOOT_LINE_LENGTH);
+ path[0] = 0;
+ if ((use_rootdev == 1) && (strlen(drivername) == 0)) {
+ strncpy(path, root_device, BOOT_LINE_LENGTH);
+ } else {
+ char buffer[32];
+ snprintf(path, BOOT_LINE_LENGTH, "%s%c", drivername, 'a'+disk);
+ if (part != -1) {
+ snprintf(buffer, 31, "%d", part);
+ strlcat(path, buffer, BOOT_LINE_LENGTH);
+ }
+ if (addr != -1) {
+ snprintf(buffer, 31, "@0x%x", addr);
+ strlcat(path, buffer, BOOT_LINE_LENGTH);
+ }
+ buffer[0]=':';
+ buffer[1]='\0';
+ strlcat(path, buffer, BOOT_LINE_LENGTH);
+ }
+ strlcat(path, arg, BOOT_LINE_LENGTH);
}
/* initrd */
Index: filo/main/grub/builtins.c
===================================================================
--- filo.orig/main/grub/builtins.c
+++ filo/main/grub/builtins.c
@@ -747,6 +747,12 @@ void copy_path_to_filo_bootline(char *ar
arg += i;
}
+ if ((disk == -1) && (part != -1) && (strlen(drivername) == 0)) {
+ // special case for partition-only identifiers:
+ // take driver and disk number from root_device
+ i = parse_linux_style_driver(root_device, drivername, &disk);
+ }
+
path[0] = 0;
if ((use_rootdev == 1) && (strlen(drivername) == 0)) {
strncpy(path, root_device, BOOT_LINE_LENGTH);
signature.asc
Description: This is a digitally signed message part
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
