Re: [PATCH 090/112] vsprintf: add %pD for printing EFI device path

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

Hello Sascha,

On 08.01.24 10:01, Sascha Hauer wrote:
> On Wed, Jan 03, 2024 at 07:12:50PM +0100, Ahmad Fatoum wrote:
>> We already have a few users that will want to print EFI device paths and
>> will gain quite a few more with incoming loader support, so let's allow
>> printing them directly with printf with the %pD format specifier.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  lib/vsprintf.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> index ea092c06d3d6..e41a1abee652 100644
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -20,6 +20,7 @@
>>  #include <kallsyms.h>
>>  #include <wchar.h>
>>  #include <of.h>
>> +#include <efi.h>
>>  
>>  #include <common.h>
>>  #include <pbl.h>
>> @@ -348,6 +349,15 @@ char *uuid_string(char *buf, const char *end, const u8 *addr, int field_width,
>>  	return string(buf, end, uuid, field_width, precision, flags);
>>  }
>>  
>> +static char *device_path_string(char *buf, const char *end, const struct efi_device_path *dp,
>> +				int field_width, int precision, int flags)
>> +{
>> +	if (!dp)
>> +		return string(buf, end, NULL, field_width, precision, flags);
>> +
>> +	return buf + device_path_to_str_buf(dp, buf, end - buf);
>> +}
>> +
>>  static noinline_for_stack
>>  char *hex_string(char *buf, const char *end, const u8 *addr, int field_width,
>>  		 int precision, int flags, const char *fmt)
>> @@ -519,6 +529,10 @@ static char *pointer(const char *fmt, char *buf, const char *end, const void *pt
>>  	case 'J':
>>  		if (fmt[1] == 'P' && IS_ENABLED(CONFIG_JSMN))
>>  			return jsonpath_string(buf, end, ptr, field_width, precision, flags, fmt);
>> +	case 'D':
>> +		if (IS_ENABLED(CONFIG_EFI_DEVICEPATH))
>> +			return device_path_string(buf, end, ptr, field_width, precision, flags);
>> +		break;
> 
> Linux uses 'D' to print a filename from a struct file. This might become
> useful for barebox as well, so we might better choose a different letter
> here.

We can use 'd' along with f->f_path.dentry for that when we need to.
Another letter may conflict in future with something else that we want.
I would prefer to use 'D' as that's what U-Boot does and it avoids needless
churn for the UEFI code.

> Otherwise I think it's ok to introduce barebox specific pointer types in
> our printf implementation, even when they conflict with ones from the
> kernel, but we should update the comment above this function accordingly
> and clearly state when a letter is barebox specific.

I can do that.

Thanks,
Ahmad

> 
> Sascha
> 

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