| Message ID | 20191120024306.16526-6-jk@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | discover/grub2: Add support for grub2 file specifiers | expand |
On Wed, 2019-11-20 at 10:42 +0800, Jeremy Kerr wrote: > Currently, we have struct grub2_resource_info to keep references to > boot > payloads that may be returned in boot options, and be (conditionally) > resolved by the parser. > > We'd also like to use the same semantics for other file references in > the grub2 parser, for arbitrary usage in scripts - where files are > also referenced by a path and an optional device. > > To do this, this change moves struct grub2_resource_info to grub2.h, > and > renames to struct grub2_file. Future changes will use this for > script-internal file handling. > > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> > --- > discover/grub2/grub2.c | 24 +++++++++--------------- > discover/grub2/grub2.h | 8 ++++++++ > 2 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/discover/grub2/grub2.c b/discover/grub2/grub2.c > index f62ccdd..412298b 100644 > --- a/discover/grub2/grub2.c > +++ b/discover/grub2/grub2.c > @@ -33,17 +33,12 @@ static const char *const grub2_conf_files[] = { > NULL > }; > > -struct grub2_resource_info { > - char *root; > - char *path; > -}; > - > /* we use slightly different resources for grub2 */ > struct resource *create_grub2_resource(struct discover_boot_option > *opt, > struct discover_device *orig_device, > const char *root, const char *path) > { > - struct grub2_resource_info *info; > + struct grub2_file *file; > struct resource *res; > > if (strstr(path, "://")) { > @@ -55,13 +50,12 @@ struct resource *create_grub2_resource(struct > discover_boot_option *opt, > res = talloc(opt, struct resource); > > if (root) { > - info = talloc(res, struct grub2_resource_info); > - talloc_reference(info, root); > - info->root = talloc_strdup(info, root); > - info->path = talloc_strdup(info, path); > + file = talloc(res, struct grub2_file); > + file->dev = talloc_strdup(file, root); > + file->path = talloc_strdup(file, path); Should be fine I think but caught my eye; do we lose anything by dropping that talloc_reference()? > > res->resolved = false; > - res->info = info; > + res->info = file; > > } else > resolve_resource_against_device(res, orig_device, > path); > @@ -72,18 +66,18 @@ struct resource *create_grub2_resource(struct > discover_boot_option *opt, > bool resolve_grub2_resource(struct device_handler *handler, > struct resource *res) > { > - struct grub2_resource_info *info = res->info; > + struct grub2_file *file = res->info; > struct discover_device *dev; > > assert(!res->resolved); > > - dev = device_lookup_by_uuid(handler, info->root); > + dev = device_lookup_by_uuid(handler, file->dev); > > if (!dev) > return false; > > - resolve_resource_against_device(res, dev, info->path); > - talloc_free(info); > + resolve_resource_against_device(res, dev, file->path); > + talloc_free(file); > > return true; > } > diff --git a/discover/grub2/grub2.h b/discover/grub2/grub2.h > index 68176fb..73d91b2 100644 > --- a/discover/grub2/grub2.h > +++ b/discover/grub2/grub2.h > @@ -107,6 +107,14 @@ struct grub2_parser { > bool inter_word; > }; > > +/* References to files in grub2 consist of an optional device and a > path > + * (specified here by UUID). If the dev is unspecified, we fall back > to a > + * default - usually the 'root' environment variable. */ > +struct grub2_file { > + char *dev; > + char *path; > +}; > + > /* type for builtin functions */ > typedef int (*grub2_function)(struct grub2_script *script, void > *data, > int argc, char *argv[]);
Hi Sam, > > @@ -55,13 +50,12 @@ struct resource *create_grub2_resource(struct > > discover_boot_option *opt, > > res = talloc(opt, struct resource); > > > > if (root) { > > - info = talloc(res, struct grub2_resource_info); > > - talloc_reference(info, root); > > - info->root = talloc_strdup(info, root); > > - info->path = talloc_strdup(info, path); > > + file = talloc(res, struct grub2_file); > > + file->dev = talloc_strdup(file, root); > > + file->path = talloc_strdup(file, path); > > Should be fine I think but caught my eye; do we lose anything by > dropping that talloc_reference()? That wasn't needed in the first place - since we talloc_strdup() root, we never needed an additional reference to it. It looks like I should have removed that in 1513dd5d0, where we changed from using the root var to the strdup. Cheers, Jeremy
diff --git a/discover/grub2/grub2.c b/discover/grub2/grub2.c index f62ccdd..412298b 100644 --- a/discover/grub2/grub2.c +++ b/discover/grub2/grub2.c @@ -33,17 +33,12 @@ static const char *const grub2_conf_files[] = { NULL }; -struct grub2_resource_info { - char *root; - char *path; -}; - /* we use slightly different resources for grub2 */ struct resource *create_grub2_resource(struct discover_boot_option *opt, struct discover_device *orig_device, const char *root, const char *path) { - struct grub2_resource_info *info; + struct grub2_file *file; struct resource *res; if (strstr(path, "://")) { @@ -55,13 +50,12 @@ struct resource *create_grub2_resource(struct discover_boot_option *opt, res = talloc(opt, struct resource); if (root) { - info = talloc(res, struct grub2_resource_info); - talloc_reference(info, root); - info->root = talloc_strdup(info, root); - info->path = talloc_strdup(info, path); + file = talloc(res, struct grub2_file); + file->dev = talloc_strdup(file, root); + file->path = talloc_strdup(file, path); res->resolved = false; - res->info = info; + res->info = file; } else resolve_resource_against_device(res, orig_device, path); @@ -72,18 +66,18 @@ struct resource *create_grub2_resource(struct discover_boot_option *opt, bool resolve_grub2_resource(struct device_handler *handler, struct resource *res) { - struct grub2_resource_info *info = res->info; + struct grub2_file *file = res->info; struct discover_device *dev; assert(!res->resolved); - dev = device_lookup_by_uuid(handler, info->root); + dev = device_lookup_by_uuid(handler, file->dev); if (!dev) return false; - resolve_resource_against_device(res, dev, info->path); - talloc_free(info); + resolve_resource_against_device(res, dev, file->path); + talloc_free(file); return true; } diff --git a/discover/grub2/grub2.h b/discover/grub2/grub2.h index 68176fb..73d91b2 100644 --- a/discover/grub2/grub2.h +++ b/discover/grub2/grub2.h @@ -107,6 +107,14 @@ struct grub2_parser { bool inter_word; }; +/* References to files in grub2 consist of an optional device and a path + * (specified here by UUID). If the dev is unspecified, we fall back to a + * default - usually the 'root' environment variable. */ +struct grub2_file { + char *dev; + char *path; +}; + /* type for builtin functions */ typedef int (*grub2_function)(struct grub2_script *script, void *data, int argc, char *argv[]);
Currently, we have struct grub2_resource_info to keep references to boot payloads that may be returned in boot options, and be (conditionally) resolved by the parser. We'd also like to use the same semantics for other file references in the grub2 parser, for arbitrary usage in scripts - where files are also referenced by a path and an optional device. To do this, this change moves struct grub2_resource_info to grub2.h, and renames to struct grub2_file. Future changes will use this for script-internal file handling. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> --- discover/grub2/grub2.c | 24 +++++++++--------------- discover/grub2/grub2.h | 8 ++++++++ 2 files changed, 17 insertions(+), 15 deletions(-)