Hollis Blanchard <[EMAIL PROTECTED]> writes: > This code sets the GRUB "prefix" environment variable from the OF > /chosen/bootpath property. It must therefore translate between the two > syntaxes.
Sven just told me bootpath does not work on the pegasos II, although it will be fixed. So the argument will still be required, I guess. I will send in a patch for that later (one that will work with this patch applied). > I believe most of this code could be moved to a kern/ieee1275/device.c > file, but that can wait until there is another supported OF platform. Right. A lot of code can be moved and shared. > I have verified that this works (a config file is loaded) from disk, and > netbooting is not broken. The subdirectory stuff works as well. Ok. I still have to test it... :/ > 2005-02-13 Hollis Blanchard <[EMAIL PROTECTED]> [...] > (GRUB_PARSE_PARTITION): Likewise. Just make this a new enum. So name the enum and say "New enum `foo'.". See my comment below. > Index: kern/powerpc/ieee1275/init.c [...] > +/* Translate an OF filesystem path (separated by backslashes), into a GRUB > + path (separated by forward slashes). */ > +static void > +translate_path (char *filepath) Can you add a prefix? > +static void > +grub_set_prefix (void) > +{ > + char bootpath[64]; /* XXX check length */ You could get the property length and use that. It would be clearer. See my patch for passing the prefix as an argument for an example of this. I think this was done like this before, but that should be changed as well. :) > Index: kern/powerpc/ieee1275/openfw.c [...] > +enum { Please put the { on a new line. > + GRUB_PARSE_FILENAME, > + GRUB_PARSE_PARTITION, > +}; I think there can be some problems with unnamed enums. I am not sure though... > + > /* Walk children of 'devpath', calling hook for each. */ > grub_err_t > grub_children_iterate (char *devpath, > @@ -64,7 +69,7 @@ grub_children_iterate (char *devpath, > if (actual == -1) > continue; > > - grub_sprintf(fullname, "%s/%s", devpath, childname); > + grub_sprintf (fullname, "%s/%s", devpath, childname); > > alias.type = childtype; > alias.path = childpath; > @@ -199,3 +204,133 @@ grub_claimmap (grub_addr_t addr, grub_si > > return 0; > } > + > +/* Get the device arguments of the Open Firmware device specifier `path'. */ > +static char * Device arguments are like the partition number or so? > +static char * > +grub_ieee1275_parse_args (const char *path, int field) > +{ > + char type[64]; /* XXX check size. */ Same stuff as before. :) > + char *device = grub_ieee1275_get_devname (path); > + char *args = grub_ieee1275_get_devargs (path); > + char *ret = 0; > + grub_ieee1275_phandle_t dev; > + > + if (!args) > + /* Shouldn't happen. */ > + return 0; If I understood your code correctly it can't happen at all, in that case the test would be useless. > + > + /* We need to know what type of device it is in order to parse the full > + file path properly. */ > + if (grub_ieee1275_finddevice (device, &dev)) > + { > + grub_error (GRUB_ERR_UNKNOWN_DEVICE, "Device %s not found\n", device); > + goto out; > + } > + if (grub_ieee1275_get_property (dev, "device_type", &type, sizeof type, 0)) > + { > + grub_error (GRUB_ERR_UNKNOWN_DEVICE, > + "Device %s lacks a device_type property\n", device); > + goto out; > + } > + > + if (!grub_strcmp ("block", type)) > + { > + /* Example: "disk:<partition number>,<file name>". */ Is it always like this without exceptions? Will this always be an alias or will this be copied from "boot"? > + ret = grub_malloc (grub_strlen (filepath) + 1); > + /* Make sure filepath has leading backslash. */ > + if (filepath[0] != '\\') > + grub_sprintf (ret, "\\%s", filepath); > + else > + grub_strcpy (ret, filepath); Why is this required? > +out: Please use "fail:", just to be consistent. > +char * > +grub_ieee1275_get_dev_encoding (const char *path) > +{ > + char *device = grub_ieee1275_get_devname (path); > + char *partition = grub_ieee1275_parse_args (path, GRUB_PARSE_PARTITION); > + char *encoding; > + > + if (partition) > + { > + unsigned int partno = grub_strtoul (partition, 0, 0); > + partno--; /* GRUB partition numbering is 0-based. */ Right. But how can you be sure both match? > + > + /* XXX Assume partno will only require two bytes? */ > + encoding = grub_malloc (grub_strlen (device) + 5); Can't you just calculate this? Otherwise you can better allocate a few bytes too much. Thanks, Marco _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel