[PATCH v2 3/3] discover/grub: Improve BLS grub environment variables expansion

[Javier Martinez Canillas]

On 06/12/2018 06:32 AM, Samuel Mendoza-Jonas wrote:
> On Thu, 2018-06-07 at 19:35 +0200, Javier Martinez Canillas wrote:
>> The fields from a BootLoaderSpec file can contain environment variables,
>> in GRUB 2 these are show verbatim and are evaluated later when an entry
>> is selected. But on Petitboot these have to be expanded before creating
>> the GRUB 2 resources and show in the UI the values after the evaluation.
>>
>> The current blscfg handler had a very limited support for variables, it
>> only had support for the options field and also didn't take into account
>> that variables could be mixed with literal values.
>>
>> So for example the following fields were not expanded correctly:
>>
>>   linux   $bootprefix/vmlinuz
>>
>>   options $kernelopts foo=bar
>>
>>   options foo=bar $kernelopts
>>
>>   options $kernelopts $debugopts
>>
>> Also change some of the tests to cover mixing variables and literals.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm at redhat.com>
>>
>> ---
>>
>>  discover/grub2/blscfg.c                      | 86 ++++++++++++++++----
>>  test/parser/test-grub2-blscfg-multiple-bls.c |  5 +-
>>  test/parser/test-grub2-blscfg-opts-grubenv.c |  4 +-
>>  3 files changed, 75 insertions(+), 20 deletions(-)
>>
>> diff --git a/discover/grub2/blscfg.c b/discover/grub2/blscfg.c
>> index a3813064a0a..fdbe2468952 100644
>> --- a/discover/grub2/blscfg.c
>> +++ b/discover/grub2/blscfg.c
>> @@ -2,6 +2,7 @@
>>  #define _GNU_SOURCE
>>  
>>  #include <assert.h>
>> +#include <ctype.h>
>>  #include <stdlib.h>
>>  #include <string.h>
>>  #include <dirent.h>
>> @@ -34,54 +35,107 @@ struct bls_state {
>>  	const char *dtb;
>>  };
>>  
>> +static char *field_append(struct bls_state *state, int type, char *buffer,
>> +			  char *start, char *end)
>> +{
>> +	char *temp = talloc_strndup(state, start, end - start + 1);
>> +	temp[end - start + 1] = '\0';
> 
> This can go one byte past the end of temp depending on what start and end
> are; _strndup() should terminate this buffer itself anyway.
>

You are right, I removed this on v3.
 
>> +	const char *field = temp;
> 
> Also mostly for looks try not to mix declarations and code :)
>

Indeed, my bad. This got fixed too by removing the line mentioned above.
 
> Cheers,
> Sam
> 
Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat