Re: [PATCH 1/2] commands: boot: refactor boot entry creation to use iterators

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 2/14/20 8:56 AM, Sascha Hauer wrote:
> On Wed, Feb 12, 2020 at 03:16:37PM +0100, Ahmad Fatoum wrote:
>> We currently create all boot entries before attempting boot. This is
>> less than optimal, because this may involve probing devices that won't
>> be used for actual boot.
>>
>> In preparation for changing this, refactor the code, so we only have one
>> loop we need to touch.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  commands/boot.c | 44 ++++++++++++++++++++++++++++----------------
>>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> Applied, thanks

Relooking at the diff, I spotted a small issue that would happen with
dryrun boot. I sent a fixup.

> 
> Sascha
> 
>>
>> diff --git a/commands/boot.c b/commands/boot.c
>> index 0257b3dd4f18..ce4eeac6534c 100644
>> --- a/commands/boot.c
>> +++ b/commands/boot.c
>> @@ -21,13 +21,29 @@
>>  
>>  #include <linux/stat.h>
>>  
>> +static char *next_argv(void *context)
>> +{
>> +	char ***argv = context;
>> +	char *next = **argv;
>> +	(*argv)++;
>> +	return next;
>> +}
>> +
>> +static char *next_word(void *context)
>> +{
>> +	return strsep(context, " ");
>> +}
>> +
>>  static int do_boot(int argc, char *argv[])
>>  {
>>  	char *freep = NULL;
>>  	int opt, ret = 0, do_list = 0, do_menu = 0;
>> -	int i, dryrun = 0, verbose = 0, timeout = -1;
>> +	int dryrun = 0, verbose = 0, timeout = -1;
>>  	struct bootentries *entries;
>>  	struct bootentry *entry;
>> +	void *handle;
>> +	const char *name;
>> +	char *(*next)(void *);
>>  
>>  	while ((opt = getopt(argc, argv, "vldmt:w:")) > 0) {
>>  		switch (opt) {
>> @@ -54,31 +70,26 @@ static int do_boot(int argc, char *argv[])
>>  		}
>>  	}
>>  
>> -	entries = bootentries_alloc();
>> -
>>  	if (optind < argc) {
>> -		for (i = optind; i < argc; i++) {
>> -			ret = bootentry_create_from_name(entries, argv[i]);
>> -			if (ret <= 0)
>> -				printf("Nothing bootable found on '%s'\n", argv[i]);
>> -	       }
>> +		handle = &argv[optind];
>> +		next = next_argv;
>>  	} else {
>>  		const char *def;
>> -		char *sep, *name;
>>  
>>  		def = getenv("global.boot.default");
>>  		if (!def)
>>  			return 0;
>>  
>> -		sep = freep = xstrdup(def);
>> +		handle = freep = xstrdup(def);
>> +		next = next_word;
>> +	}
>>  
>> -		while ((name = strsep(&sep, " ")) != NULL) {
>> -			ret = bootentry_create_from_name(entries, name);
>> -			if (ret <= 0)
>> -				printf("Nothing bootable found on '%s'\n", name);
>> -		}
>> +	entries = bootentries_alloc();
>>  
>> -		free(freep);
>> +	while ((name = next(&handle)) != NULL) {
>> +		ret = bootentry_create_from_name(entries, name);
>> +		if (ret <= 0)
>> +			printf("Nothing bootable found on '%s'\n", name);
>>  	}
>>  
>>  	if (list_empty(&entries->entries)) {
>> @@ -104,6 +115,7 @@ static int do_boot(int argc, char *argv[])
>>  
>>  out:
>>  	bootentries_free(entries);
>> +	free(freep);
>>  
>>  	return ret;
>>  }
>> -- 
>> 2.25.0
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>>
> 

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox