[PATCH] discover/grub: Allow to set a default index for BLS entries

Javier Martinez Canillas javierm at redhat.com
Wed Apr 18 02:33:51 AEST 2018


On 04/17/2018 06:40 AM, Samuel Mendoza-Jonas wrote:
> On Thu, 2018-04-05 at 14:16 +0200, Javier Martinez Canillas wrote:

[snip]

>>  
>> -static bool option_is_default(struct grub2_script *script,
>> +static bool option_is_default(struct bls_state *state,
>>  			      struct boot_option *option)
>>  {
>> +	unsigned int idx;
>>  	const char *var;
>> +	char *end;
>>  
>> -	var = script_env_get(script, "default");
>> +	var = script_env_get(state->script, "default");
>>  	if (!var)
>>  		return false;
>>  
>>  	if (!strcmp(var, option->id))
>>  		return true;
>>  
>> -	return !strcmp(var, option->name);
>> +	if (!strcmp(var, option->name))
>> +		return true;
>> +
>> +	idx = strtoul(var, &end, 10);
>> +	return !!(end != var && *end == '\0' && idx == state->idx);
> 
> I think we don't need the !! here?
> 

Yeah, I know. I usually use it to make clear that's a boolean, but
I agree that makes the code excessively verbose for no good reason.

>>  }
>>  
>>  static void bls_finish(struct conf_context *conf)
>> @@ -141,7 +148,7 @@ static void bls_finish(struct conf_context *conf)
>>  		opt->dtb = create_grub2_resource(opt, conf->dc->device,
>>  						 root, state->dtb);
>>  
>> -	option->is_default = option_is_default(state->script, option);
>> +	option->is_default = option_is_default(state, option);
>>  
>>  	discover_context_add_boot_option(dc, opt);
>>  
>> @@ -179,6 +186,7 @@ int builtin_blscfg(struct grub2_script *script,
>>  	struct dirent **bls_entries;
>>  	struct conf_context *conf;
>>  	struct bls_state *state;
>> +	unsigned int current_idx = 0;
> 
> Without trying to over-complicate things, will a blscfg call in grub.cfg
> only occur before normal GRUB boot options or can it be in between or
> after? If the latter then would we need to set current_idx to the number
> of boot options already parsed from this grub.cfg?
> 

I'll fix this and also change the grub2-blscfg-default-index test case to
have both menu entries in the grub config and BLS fragments.

Something I noticed now when mixing them is that the Petitboot UI (at least
the ncurses based one) sorts the boot entries in the reverse order than I
would had expected. That is, the entry at the top is the older one and the
entry at the bottom is the latest one.

But both in the grub.cfg file and the grub UI, the boot menu entries are in
a descending order from newer to older. I wrongly assumed that was the case
in Petitboot as well. I'll include a fix so the menu entries are sorted as
expected by the Petitboot UI.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat


More information about the Petitboot mailing list