Re: [PATCH 1/3] blspec: refactor to prepare for booting by file path

[Sascha Hauer <sha@pengutronix.de>]

On Sun, Jul 24, 2022 at 08:48:05PM +0200, Ahmad Fatoum wrote:
> The blspec boot entry provider currently feeds the path into
> blspec_scan_directory, which will fail to collect an boot entry when
> given a regular file. In preparation for allowing to boot a specific
> blspec file by path, refactor the code pertaining to a single file into
> a blspec_scan_file function and move parse_nfs_url which applies equally
> to files and directories into the callsite. There is one other callsite
> of blspec_scan_directory in blspec_scan_cdev, but that one will never
> handle NFS paths, so it's fine to skip calling parse_nfs_url there.
> 
> No functional change intended.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  common/blspec.c  | 112 ++++++++++++++++++++++++-----------------------
>  include/blspec.h |   2 +
>  2 files changed, 60 insertions(+), 54 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/common/blspec.c b/common/blspec.c
> index 158fd1e9a28c..d798f6f38a4b 100644
> --- a/common/blspec.c
> +++ b/common/blspec.c
> @@ -513,6 +513,54 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
>  	return ret;
>  }
>  
> +int blspec_scan_file(struct bootentries *bootentries, const char *root,
> +		     const char *configname)
> +{
> +	char *devname = NULL, *hwdevname = NULL;
> +	struct blspec_entry *entry;
> +
> +	if (blspec_have_entry(bootentries, configname))
> +		return -EEXIST;
> +
> +	entry = blspec_entry_open(bootentries, configname);
> +	if (IS_ERR(entry))
> +		return PTR_ERR(entry);
> +
> +	entry->rootpath = xstrdup(root);
> +	entry->configpath = xstrdup(configname);
> +	entry->cdev = get_cdev_by_mountpath(root);
> +
> +	if (!entry_is_of_compatible(entry)) {
> +		blspec_entry_free(&entry->entry);
> +		return -ENODEV;
> +	}
> +
> +	if (!entry_is_match_machine_id(entry)) {
> +		blspec_entry_free(&entry->entry);
> +		return -ENODEV;
> +	}
> +
> +	if (entry->cdev && entry->cdev->dev) {
> +		devname = xstrdup(dev_name(entry->cdev->dev));
> +		if (entry->cdev->dev->parent)
> +			hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
> +	}
> +
> +	entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
> +				       configname);
> +	entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
> +					    devname ? devname : "none",
> +					    hwdevname ? hwdevname : "none");
> +	free(devname);
> +	free(hwdevname);
> +
> +	entry->entry.me.type = MENU_ENTRY_NORMAL;
> +	entry->entry.release = blspec_entry_free;
> +
> +	bootentries_add_entry(bootentries, &entry->entry);
> +	return 1;
> +}
> +
>  /*
>   * blspec_scan_directory - scan over a directory
>   *
> @@ -522,17 +570,11 @@ static bool entry_is_match_machine_id(struct blspec_entry *entry)
>   */
>  int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  {
> -	struct blspec_entry *entry;
>  	DIR *dir;
>  	struct dirent *d;
>  	char *abspath;
>  	int ret, found = 0;
>  	const char *dirname = "loader/entries";
> -	char *nfspath = NULL;
> -
> -	nfspath = parse_nfs_url(root);
> -	if (!IS_ERR(nfspath))
> -		root = nfspath;
>  
>  	pr_debug("%s: %s %s\n", __func__, root, dirname);
>  
> @@ -549,7 +591,6 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  		char *configname;
>  		struct stat s;
>  		char *dot;
> -		char *devname = NULL, *hwdevname = NULL;
>  
>  		if (*d->d_name == '.')
>  			continue;
> @@ -578,59 +619,15 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
>  			continue;
>  		}
>  
> -		if (blspec_have_entry(bootentries, configname)) {
> -			free(configname);
> -			continue;
> -		}
> -
> -		entry = blspec_entry_open(bootentries, configname);
> -		if (IS_ERR(entry)) {
> -			free(configname);
> -			continue;
> -		}
> -
> -		entry->rootpath = xstrdup(root);
> -		entry->configpath = configname;
> -		entry->cdev = get_cdev_by_mountpath(root);
> -
> -		if (!entry_is_of_compatible(entry)) {
> -			blspec_entry_free(&entry->entry);
> -			continue;
> -		}
> -
> -		if (!entry_is_match_machine_id(entry)) {
> -			blspec_entry_free(&entry->entry);
> -			continue;
> -		}
> -
> -		found++;
> -
> -		if (entry->cdev && entry->cdev->dev) {
> -			devname = xstrdup(dev_name(entry->cdev->dev));
> -			if (entry->cdev->dev->parent)
> -				hwdevname = xstrdup(dev_name(entry->cdev->dev->parent));
> -		}
> -
> -		entry->entry.title = xasprintf("%s (%s)", blspec_entry_var_get(entry, "title"),
> -					       configname);
> -		entry->entry.description = basprintf("blspec entry, device: %s hwdevice: %s",
> -						    devname ? devname : "none",
> -						    hwdevname ? hwdevname : "none");
> -		free(devname);
> -		free(hwdevname);
> -
> -		entry->entry.me.type = MENU_ENTRY_NORMAL;
> -		entry->entry.release = blspec_entry_free;
> -
> -		bootentries_add_entry(bootentries, &entry->entry);
> +		ret = blspec_scan_file(bootentries, root, configname);
> +		if (ret == 1)
> +			found++;
>  	}
>  
>  	ret = found;
>  
>  	closedir(dir);
>  err_out:
> -	if (!IS_ERR(nfspath))
> -		free(nfspath);
>  	free(abspath);
>  
>  	return ret;
> @@ -846,9 +843,16 @@ static int blspec_bootentry_provider(struct bootentries *bootentries,
>  		found += ret;
>  
>  	if (*name == '/' || !strncmp(name, "nfs://", 6)) {
> +		char *nfspath = parse_nfs_url(name);
> +		if (!IS_ERR(nfspath))
> +			name = nfspath;
> +
>  		ret = blspec_scan_directory(bootentries, name);
>  		if (ret > 0)
>  			found += ret;
> +
> +		if (!IS_ERR(nfspath))
> +			free(nfspath);
>  	}
>  
>  	return found;
> diff --git a/include/blspec.h b/include/blspec.h
> index 37076cd47c97..8f2556f6620e 100644
> --- a/include/blspec.h
> +++ b/include/blspec.h
> @@ -23,5 +23,7 @@ int blspec_scan_devices(struct bootentries *bootentries);
>  int blspec_scan_device(struct bootentries *bootentries, struct device_d *dev);
>  int blspec_scan_devicename(struct bootentries *bootentries, const char *devname);
>  int blspec_scan_directory(struct bootentries *bootentries, const char *root);
> +int blspec_scan_file(struct bootentries *bootentries, const char *root,
> +		     const char *configname);
>  
>  #endif /* __LOADER_H__ */
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |