[PATCH] env: Fix warning when forcing environment without ENV_ACCESS_IGNORE_FORCE

Marek Vasut marex at denx.de
Thu Jan 28 19:59:56 CET 2021


On 1/28/21 10:11 AM, Heinrich Schuchardt wrote:
[...]
>> diff --git a/env/flags.c b/env/flags.c
>> index df4aed2..e3e833c 100644
>> --- a/env/flags.c
>> +++ b/env/flags.c
>> @@ -563,12 +563,13 @@ int env_flags_validate(const struct env_entry *item, const char *newval,
> 
> Please, add a Sphinx style function description in include/env_flags.h
> explaining exactly what this function is meant to do.
> 
> Cf.
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

This is irrelevant, it's a bugfix so docs/improvements are a separate patch.

>>   		return 1;
>>   #endif
>>
>> -#ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
>>   	if (flag & H_FORCE) {
>> +#ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
>>   		printf("## Error: Can't force access to \"%s\"\n", name);
> 
> If this is an error, why don't you return 1 (failure)?
> Please, use log_err() for error messages.
> 
>> +#else
>>   		return 0;
> 
> Why shouldn't the other tests be executed?

Because the code is a total mess and thus error prone.


More information about the U-Boot mailing list