| Message ID | 20171118110946.26014-1-xypron.glpk@gmx.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | [U-Boot,1/1] vsprintf.c: add EFI device path printing | expand |
Dear Heinrich, In message <20171118110946.26014-1-xypron.glpk@gmx.de> you wrote: > For debugging efi_loader we need the capability to print EFI > device paths. With this patch we can write: > > debug("device path: %pD", dp); ... > +#ifdef CONFIG_EFI_LOADER > +static char *device_path_string(char *buf, char *end, void *dp, int field_width, > + int precision, int flags) > +{ > + u16 *str = efi_dp_str((struct efi_device_path *)dp); > + > + buf = string16(buf, end, str, field_width, precision, flags); > + efi_free_pool(str); efi_dp_str() can return NULL. Should thhis not be handled? Best regards, Wolfgang Denk
On 11/19/2017 12:41 PM, Wolfgang Denk wrote: > Dear Heinrich, > > In message <20171118110946.26014-1-xypron.glpk@gmx.de> you wrote: >> For debugging efi_loader we need the capability to print EFI >> device paths. With this patch we can write: >> >> debug("device path: %pD", dp); > ... > >> +#ifdef CONFIG_EFI_LOADER >> +static char *device_path_string(char *buf, char *end, void *dp, int field_width, >> + int precision, int flags) >> +{ >> + u16 *str = efi_dp_str((struct efi_device_path *)dp); >> + >> + buf = string16(buf, end, str, field_width, precision, flags); >> + efi_free_pool(str); > > efi_dp_str() can return NULL. Should this not be handled? Thanks for reviewing. This situation is caught in string16: u16 *str = s ? s : L"<NULL>"; It can only occur if we are out of memory. All other error handling should be added to efi_convert_device_path_to_text(). Best regards Heinrich
Dear Heinrich, In message <452bd166-5cc7-ff1d-925c-dc8b8e1bdbe5@gmx.de> you wrote: > > >> +#ifdef CONFIG_EFI_LOADER > >> +static char *device_path_string(char *buf, char *end, void *dp, int field_width, > >> + int precision, int flags) > >> +{ > >> + u16 *str = efi_dp_str((struct efi_device_path *)dp); > >> + > >> + buf = string16(buf, end, str, field_width, precision, flags); > >> + efi_free_pool(str); > > > > efi_dp_str() can return NULL. Should this not be handled? > > Thanks for reviewing. > > This situation is caught in string16: > u16 *str = s ? s : L"<NULL>"; This is crash-preventing cosmetics, but no real error handling. I mean, should we not wave a big red flag and shout at the user: "Hey, your system is going berserk here!" ? > It can only occur if we are out of memory. All other error handling > should be added to efi_convert_device_path_to_text(). Yes, and running out of memory at such a place is likely to be unrecoverable. So we should terminate all further processing baed on this result, and not continue with bogus data. Best regards, Wolfgang Denk
On 11/20/2017 04:44 PM, Wolfgang Denk wrote: > Dear Heinrich, > > In message <452bd166-5cc7-ff1d-925c-dc8b8e1bdbe5@gmx.de> you wrote: >> >>>> +#ifdef CONFIG_EFI_LOADER >>>> +static char *device_path_string(char *buf, char *end, void *dp, int field_width, >>>> + int precision, int flags) >>>> +{ >>>> + u16 *str = efi_dp_str((struct efi_device_path *)dp); >>>> + >>>> + buf = string16(buf, end, str, field_width, precision, flags); >>>> + efi_free_pool(str); >>> >>> efi_dp_str() can return NULL. Should this not be handled? >> >> Thanks for reviewing. >> >> This situation is caught in string16: >> u16 *str = s ? s : L"<NULL>"; > > This is crash-preventing cosmetics, but no real error handling. I > mean, should we not wave a big red flag and shout at the user: "Hey, > your system is going berserk here!" ? Why do you think that the error handling should be in THIS function? I can understand that we might improve error handling in the EFI coding but I do not believe we should do it here. > >> It can only occur if we are out of memory. All other error handling >> should be added to efi_convert_device_path_to_text(). > > Yes, and running out of memory at such a place is likely to be > unrecoverable. So we should terminate all further processing baed on > this result, and not continue with bogus data. printf() does not have any return value. So here we could only panic(). Is this really what you have in mind? Best regards Heinrich
Dear Heinrich Schuchardt, In message <d643b22b-2511-9b40-772b-5f7f4486c289@gmx.de> you wrote: > > >>>> + u16 *str = efi_dp_str((struct efi_device_path *)dp); > >>>> + > >>>> + buf = string16(buf, end, str, field_width, precision, flags); > >>>> + efi_free_pool(str); > >>> > >>> efi_dp_str() can return NULL. Should this not be handled? > >> > >> Thanks for reviewing. > >> > >> This situation is caught in string16: > >> u16 *str = s ? s : L"<NULL>"; > > > > This is crash-preventing cosmetics, but no real error handling. I > > mean, should we not wave a big red flag and shout at the user: "Hey, > > your system is going berserk here!" ? > > Why do you think that the error handling should be in THIS function? It should be somewhere - but you cannot handle an error condition that you don't report to the caller. Instead of returning from the function with a clear error message and an error return code, you ignore it. The minimum action to take here would be that device_path_string() returns NULL when the error occurs, if there was a chance for the caller to handle that. > I can understand that we might improve error handling in the EFI coding > but I do not believe we should do it here. But if you don't even return an error code you have no chance to handle it elsewhere. > > Yes, and running out of memory at such a place is likely to be > > unrecoverable. So we should terminate all further processing baed on > > this result, and not continue with bogus data. > > printf() does not have any return value. This is incorrect. printf() is declared as int printf(const char *fmt, ...); as usual. > So here we could only panic(). > > Is this really what you have in mind? Well, which other options do you see when you run out of memory? I can't see how you could recover from this, so if you don't abort here with a clear error message you will either do the same elsewhere (with a misleading error, as the actual problem happened eventually long before), or you will just crash, or run into undefined behaviour. Ignoring error condistions is always a Bad thing (TM). Best regards, Wolfgang Denk
On 11/21/2017 03:16 PM, Wolfgang Denk wrote: > Dear Heinrich Schuchardt, > > In message <d643b22b-2511-9b40-772b-5f7f4486c289@gmx.de> you wrote: >> >>>>>> + u16 *str = efi_dp_str((struct efi_device_path *)dp); >>>>>> + >>>>>> + buf = string16(buf, end, str, field_width, precision, flags); >>>>>> + efi_free_pool(str); >>>>> >>>>> efi_dp_str() can return NULL. Should this not be handled? >>>> >>>> Thanks for reviewing. >>>> >>>> This situation is caught in string16: >>>> u16 *str = s ? s : L"<NULL>"; >>> >>> This is crash-preventing cosmetics, but no real error handling. I >>> mean, should we not wave a big red flag and shout at the user: "Hey, >>> your system is going berserk here!" ? >> >> Why do you think that the error handling should be in THIS function? > > It should be somewhere - but you cannot handle an error condition > that you don't report to the caller. > > Instead of returning from the function with a clear error message > and an error return code, you ignore it. > > The minimum action to take here would be that device_path_string() > returns NULL when the error occurs, if there was a chance for the > caller to handle that. > >> I can understand that we might improve error handling in the EFI coding >> but I do not believe we should do it here. > > But if you don't even return an error code you have no chance to > handle it elsewhere. > >>> Yes, and running out of memory at such a place is likely to be >>> unrecoverable. So we should terminate all further processing baed on >>> this result, and not continue with bogus data. >> >> printf() does not have any return value. > > This is incorrect. printf() is declared as > > int printf(const char *fmt, ...); > > as usual. You are absolutely right. The C standard defines printf as returning a negative number if an error arises. Unfortunately the consumers of printf behave quite differently: xasprintf(), PyString_FromFormat() correctly identify negative values as an error. set_config_filename, dbg_snprintf_key(), bootstage_mark_code() - to name a few - will access illegal memory addresses. As long as we cannot assure that each and every caller of a printf function handles negative return values correctly the only safe handling of errors is to return 0 or panic(). > >> So here we could only panic(). >> >> Is this really what you have in mind? > > Well, which other options do you see when you run out of memory? > I can't see how you could recover from this, so if you don't abort > here with a clear error message you will either do the same > elsewhere (with a misleading error, as the actual problem happened > eventually long before), or you will just crash, or run into > undefined behaviour. So I will resend the patch with panic(). Best regards Heinrich > > Ignoring error condistions is always a Bad thing (TM). > > Best regards, > > Wolfgang Denk >
Hi Heinrich, On 23 November 2017 at 14:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 11/21/2017 03:16 PM, Wolfgang Denk wrote: >> Dear Heinrich Schuchardt, >> >> In message <d643b22b-2511-9b40-772b-5f7f4486c289@gmx.de> you wrote: >>> >>>>>>> + u16 *str = efi_dp_str((struct efi_device_path *)dp); >>>>>>> + >>>>>>> + buf = string16(buf, end, str, field_width, precision, flags); >>>>>>> + efi_free_pool(str); >>>>>> >>>>>> efi_dp_str() can return NULL. Should this not be handled? >>>>> >>>>> Thanks for reviewing. >>>>> >>>>> This situation is caught in string16: >>>>> u16 *str = s ? s : L"<NULL>"; >>>> >>>> This is crash-preventing cosmetics, but no real error handling. I >>>> mean, should we not wave a big red flag and shout at the user: "Hey, >>>> your system is going berserk here!" ? >>> >>> Why do you think that the error handling should be in THIS function? >> >> It should be somewhere - but you cannot handle an error condition >> that you don't report to the caller. >> >> Instead of returning from the function with a clear error message >> and an error return code, you ignore it. >> >> The minimum action to take here would be that device_path_string() >> returns NULL when the error occurs, if there was a chance for the >> caller to handle that. >> >>> I can understand that we might improve error handling in the EFI coding >>> but I do not believe we should do it here. >> >> But if you don't even return an error code you have no chance to >> handle it elsewhere. >> >>>> Yes, and running out of memory at such a place is likely to be >>>> unrecoverable. So we should terminate all further processing baed on >>>> this result, and not continue with bogus data. >>> >>> printf() does not have any return value. >> >> This is incorrect. printf() is declared as >> >> int printf(const char *fmt, ...); >> >> as usual. > > You are absolutely right. The C standard defines printf as returning a > negative number if an error arises. > > Unfortunately the consumers of printf behave quite differently: > > xasprintf(), PyString_FromFormat() correctly identify negative values as > an error. > > set_config_filename, dbg_snprintf_key(), bootstage_mark_code() - to name > a few - will access illegal memory addresses. I think you are tarring with too broad a brush. I don't see how sprintf() can return an error. What would an 'output error' be in that case? > > As long as we cannot assure that each and every caller of a printf > function handles negative return values correctly the only safe handling > of errors is to return 0 or panic(). I suppose that is OK, but we should try to return the expected error from standard functions. If that causes other code to fail, then we need to fix that other code. > >> >>> So here we could only panic(). >>> >>> Is this really what you have in mind? >> >> Well, which other options do you see when you run out of memory? >> I can't see how you could recover from this, so if you don't abort >> here with a clear error message you will either do the same >> elsewhere (with a misleading error, as the actual problem happened >> eventually long before), or you will just crash, or run into >> undefined behaviour. > > So I will resend the patch with panic(). > > Best regards > > Heinrich > >> >> Ignoring error condistions is always a Bad thing (TM). >> >> Best regards, >> >> Wolfgang Denk Regards, Simon
On 11/25/2017 11:34 PM, Simon Glass wrote: > Hi Heinrich, > > On 23 November 2017 at 14:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> On 11/21/2017 03:16 PM, Wolfgang Denk wrote: >>> Dear Heinrich Schuchardt, >>> >>> In message <d643b22b-2511-9b40-772b-5f7f4486c289@gmx.de> you wrote: >>>> >>>>>>>> + u16 *str = efi_dp_str((struct efi_device_path *)dp); >>>>>>>> + >>>>>>>> + buf = string16(buf, end, str, field_width, precision, flags); >>>>>>>> + efi_free_pool(str); >>>>>>> >>>>>>> efi_dp_str() can return NULL. Should this not be handled? >>>>>> >>>>>> Thanks for reviewing. >>>>>> >>>>>> This situation is caught in string16: >>>>>> u16 *str = s ? s : L"<NULL>"; >>>>> >>>>> This is crash-preventing cosmetics, but no real error handling. I >>>>> mean, should we not wave a big red flag and shout at the user: "Hey, >>>>> your system is going berserk here!" ? >>>> >>>> Why do you think that the error handling should be in THIS function? >>> >>> It should be somewhere - but you cannot handle an error condition >>> that you don't report to the caller. >>> >>> Instead of returning from the function with a clear error message >>> and an error return code, you ignore it. >>> >>> The minimum action to take here would be that device_path_string() >>> returns NULL when the error occurs, if there was a chance for the >>> caller to handle that. >>> >>>> I can understand that we might improve error handling in the EFI coding >>>> but I do not believe we should do it here. >>> >>> But if you don't even return an error code you have no chance to >>> handle it elsewhere. >>> >>>>> Yes, and running out of memory at such a place is likely to be >>>>> unrecoverable. So we should terminate all further processing baed on >>>>> this result, and not continue with bogus data. >>>> >>>> printf() does not have any return value. >>> >>> This is incorrect. printf() is declared as >>> >>> int printf(const char *fmt, ...); >>> >>> as usual. >> >> You are absolutely right. The C standard defines printf as returning a >> negative number if an error arises. >> >> Unfortunately the consumers of printf behave quite differently: >> >> xasprintf(), PyString_FromFormat() correctly identify negative values as >> an error. >> >> set_config_filename, dbg_snprintf_key(), bootstage_mark_code() - to name >> a few - will access illegal memory addresses. > > I think you are tarring with too broad a brush. I don't see how > sprintf() can return an error. What would an 'output error' be in that > case There are two sides of you question: 1) Does the printing function have a return type that can be used to signal an error? 2) Can the printing function called with an argument that can result in an error? 1) In lib/vsprintf all printing functions return int. So we could return a negative number if an error situation arises. 2) Currently we ignore error situations: %ps replaces illegal code points by question marks. If we would follow Wolfgang's reasoning we should create an error for sprintf("%ps\n", ptr) if ptr contains an unconvertible code. > >> >> As long as we cannot assure that each and every caller of a printf >> function handles negative return values correctly the only safe handling >> of errors is to return 0 or panic(). > > I suppose that is OK, but we should try to return the expected error > from standard functions. If that causes other code to fail, then we > need to fix that other code. Does this imply: as none of the current conversion functions in lib/vsprintf.c is creating an error value we only have to care about future callers printing device paths? Best regards Heinrich > >> >>> >>>> So here we could only panic(). >>>> >>>> Is this really what you have in mind? >>> >>> Well, which other options do you see when you run out of memory? >>> I can't see how you could recover from this, so if you don't abort >>> here with a clear error message you will either do the same >>> elsewhere (with a misleading error, as the actual problem happened >>> eventually long before), or you will just crash, or run into >>> undefined behaviour. >> >> So I will resend the patch with panic(). >> >> Best regards >> >> Heinrich >> >>> >>> Ignoring error condistions is always a Bad thing (TM). >>> >>> Best regards, >>> >>> Wolfgang Denk > > Regards, > Simon >
Hi Heinrich, On 25 November 2017 at 22:00, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > On 11/25/2017 11:34 PM, Simon Glass wrote: >> >> Hi Heinrich, >> >> On 23 November 2017 at 14:29, Heinrich Schuchardt <xypron.glpk@gmx.de> >> wrote: >>> >>> On 11/21/2017 03:16 PM, Wolfgang Denk wrote: >>>> >>>> Dear Heinrich Schuchardt, >>>> >>>> In message <d643b22b-2511-9b40-772b-5f7f4486c289@gmx.de> you wrote: >>>>> >>>>> >>>>>>>>> + u16 *str = efi_dp_str((struct efi_device_path *)dp); >>>>>>>>> + >>>>>>>>> + buf = string16(buf, end, str, field_width, precision, >>>>>>>>> flags); >>>>>>>>> + efi_free_pool(str); >>>>>>>> >>>>>>>> >>>>>>>> efi_dp_str() can return NULL. Should this not be handled? >>>>>>> >>>>>>> >>>>>>> Thanks for reviewing. >>>>>>> >>>>>>> This situation is caught in string16: >>>>>>> u16 *str = s ? s : L"<NULL>"; >>>>>> >>>>>> >>>>>> This is crash-preventing cosmetics, but no real error handling. I >>>>>> mean, should we not wave a big red flag and shout at the user: "Hey, >>>>>> your system is going berserk here!" ? >>>>> >>>>> >>>>> Why do you think that the error handling should be in THIS function? >>>> >>>> >>>> It should be somewhere - but you cannot handle an error condition >>>> that you don't report to the caller. >>>> >>>> Instead of returning from the function with a clear error message >>>> and an error return code, you ignore it. >>>> >>>> The minimum action to take here would be that device_path_string() >>>> returns NULL when the error occurs, if there was a chance for the >>>> caller to handle that. >>>> >>>>> I can understand that we might improve error handling in the EFI coding >>>>> but I do not believe we should do it here. >>>> >>>> >>>> But if you don't even return an error code you have no chance to >>>> handle it elsewhere. >>>> >>>>>> Yes, and running out of memory at such a place is likely to be >>>>>> unrecoverable. So we should terminate all further processing baed on >>>>>> this result, and not continue with bogus data. >>>>> >>>>> >>>>> printf() does not have any return value. >>>> >>>> >>>> This is incorrect. printf() is declared as >>>> >>>> int printf(const char *fmt, ...); >>>> >>>> as usual. >>> >>> >>> You are absolutely right. The C standard defines printf as returning a >>> negative number if an error arises. >>> >>> Unfortunately the consumers of printf behave quite differently: >>> >>> xasprintf(), PyString_FromFormat() correctly identify negative values as >>> an error. >>> >>> set_config_filename, dbg_snprintf_key(), bootstage_mark_code() - to name >>> a few - will access illegal memory addresses. >> >> >> I think you are tarring with too broad a brush. I don't see how >> sprintf() can return an error. What would an 'output error' be in that >> case > > There are two sides of you question: > > 1) Does the printing function have a return type that can be used to signal > an error? > 2) Can the printing function called with an argument that can result in an > error? > > 1) In lib/vsprintf all printing functions return int. So we could return a > negative number if an error situation arises. Yes. > > 2) Currently we ignore error situations: > %ps replaces illegal code points by question marks. > If we would follow Wolfgang's reasoning we should create an error for > sprintf("%ps\n", ptr) if ptr contains an unconvertible code. I don't think you need to take that view. The function is free to define what it does in this case. The C library man page refers to 'output error' which I take to mean that sprintf() would never generate an error. While in this case it is possible, in general, printf() cannot check its args at run-time. > >> >>> >>> As long as we cannot assure that each and every caller of a printf >>> function handles negative return values correctly the only safe handling >>> of errors is to return 0 or panic(). >> >> >> I suppose that is OK, but we should try to return the expected error >> from standard functions. If that causes other code to fail, then we >> need to fix that other code. > > > Does this imply: > > as none of the current conversion functions in lib/vsprintf.c is creating an > error value we only have to care about future callers printing device paths? You could add this to the docs for sprintf(). I think it is unfortunate that we have to alloc memory in a printf call. Is it possible to use the supplied buffer? Perhaps the EFI function should provide this option. In general, EFI code does way too much memory allocation in U-Boot. I recall seeing hundreds of memory allocations just while it was [..[ Regards,Simon
Dear Heinrich, In message <3e2e7f27-2fb6-97ad-bbab-f014b7ef684b@gmx.de> you wrote: > > You are absolutely right. The C standard defines printf as returning a > negative number if an error arises. This is what we (what to) have in U-Boot, too. > set_config_filename, dbg_snprintf_key(), bootstage_mark_code() - to name > a few - will access illegal memory addresses. In this case you have identified a number of bugs, that need fixing. > As long as we cannot assure that each and every caller of a printf > function handles negative return values correctly the only safe handling > of errors is to return 0 or panic(). Come on, be serious. Of course we MUST assume that all callaers of a function behave according o the specs. If they don;t then need fixing. But you must NEVEr change behaviour of any code to deviate from the spec and documention just because you fear thay you lose bug compatibility. That would be crazy. Best regards, Wolfgang Denk
On 11/26/2017 02:11 PM, Wolfgang Denk wrote: > Dear Heinrich, > > In message <3e2e7f27-2fb6-97ad-bbab-f014b7ef684b@gmx.de> you wrote: >> >> You are absolutely right. The C standard defines printf as returning a >> negative number if an error arises. > > This is what we (what to) have in U-Boot, too. > >> set_config_filename, dbg_snprintf_key(), bootstage_mark_code() - to name >> a few - will access illegal memory addresses. > > In this case you have identified a number of bugs, that need fixing. > >> As long as we cannot assure that each and every caller of a printf >> function handles negative return values correctly the only safe handling >> of errors is to return 0 or panic(). > > Come on, be serious. Of course we MUST assume that all callaers of > a function behave according o the specs. If they don;t then need > fixing. But you must NEVEr change behaviour of any code to deviate > from the spec and documention just because you fear thay you lose > bug compatibility. That would be crazy. > > > Best regards, > > Wolfgang Denk > Hello Wolfgang, to cut it short, would you prefer https://patchwork.ozlabs.org/patch/840912/ or https://github.com/xypron/u-boot-odroid-c2/blob/qemu-x86_64/patch/0001-vsprintf.c-add-EFI-device-path-printing.patch Best regards Heinrich
Dear Heinrich, In message <a6b8e055-5c8a-7a95-8959-1bd6958a12d6@gmx.de> you wrote: > > %ps replaces illegal code points by question marks. > If we would follow Wolfgang's reasoning we should create an error for > sprintf("%ps\n", ptr) if ptr contains an unconvertible code. You misinterpret me. the behaviour / output of *printf() for incorrect arguments etc. depends on the specification of the API. It may result in special output (like "<NULL>" for a null pointer reference). But we should always return an error for unrecoverable error situations that prevented proper operation accourding to the spec, like out-of-memory situations. > Does this imply: > > as none of the current conversion functions in lib/vsprintf.c is > creating an error value we only have to care about future callers > printing device paths? No. Whener we run into bugs or bad code, we should fix it. Ot at least flag it, so it can be fixed later. We shall never, never ever paper over known issues. Best regards, Wolfgang Denk
Dear Heinrich, In message <55e69128-b30c-78af-7e96-3aba873dfd73@gmx.de> you wrote: > > to cut it short, would you prefer > > https://patchwork.ozlabs.org/patch/840912/ > > or > > https://github.com/xypron/u-boot-odroid-c2/blob/qemu-x86_64/patch/0001-vsprintf.c-add-EFI-device-path-printing.patch Urgh. I really hate such external references. IIUC, then [2] has as only difference an additional early return in printf(). The only effect of this is to skip the "puts(printbuffer);". I can't say if this is worth the effort. In any case, you introduce inconsistent behaviour here. If you do it in printf(), it should (at least) also be done in vprintf() [and probably one should check the *snprintf() functions, too]. Best regards, Wolfgang Denk
On 11/26/2017 02:39 PM, Wolfgang Denk wrote: > Dear Heinrich, > > In message <55e69128-b30c-78af-7e96-3aba873dfd73@gmx.de> you wrote: >> >> to cut it short, would you prefer >> >> https://patchwork.ozlabs.org/patch/840912/ >> >> or >> >> https://github.com/xypron/u-boot-odroid-c2/blob/qemu-x86_64/patch/0001-vsprintf.c-add-EFI-device-path-printing.patch > > Urgh. I really hate such external references. > > IIUC, then [2] has as only difference an additional early return in > printf(). The only effect of this is to skip the > "puts(printbuffer);". 1 panics, 2 returns an error code and does not panic > > I can't say if this is worth the effort. In any case, you introduce > inconsistent behaviour here. If you do it in printf(), it should > (at least) also be done in vprintf() [and probably one should check > the *snprintf() functions, too]. Yes you are right concerning vsprintf. puts would output the incorrect data from the internal buffer. The s*printf functions work on external buffer. So the caller can react on the return code. Regards Heinrich
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index dd572d2868..68797c672c 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -18,6 +18,7 @@ #include <common.h> #include <charset.h> +#include <efi_loader.h> #include <uuid.h> #include <div64.h> @@ -292,6 +293,18 @@ static char *string16(char *buf, char *end, u16 *s, int field_width, return buf; } +#ifdef CONFIG_EFI_LOADER +static char *device_path_string(char *buf, char *end, void *dp, int field_width, + int precision, int flags) +{ + u16 *str = efi_dp_str((struct efi_device_path *)dp); + + buf = string16(buf, end, str, field_width, precision, flags); + efi_free_pool(str); + return buf; +} +#endif + #ifdef CONFIG_CMD_NET static const char hex_asc[] = "0123456789abcdef"; #define hex_asc_lo(x) hex_asc[((x) & 0x0f)] @@ -435,6 +448,11 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, #endif switch (*fmt) { +#ifdef CONFIG_EFI_LOADER + case 'D': + return device_path_string(buf, end, ptr, field_width, + precision, flags); +#endif #ifdef CONFIG_CMD_NET case 'a': flags |= SPECIAL | ZEROPAD;
For debugging efi_loader we need the capability to print EFI device paths. With this patch we can write: debug("device path: %pD", dp); A possible output would be device path: /MemoryMapped(0x0,0x3ff93a82,0x3ff93a82) Suggested-by: Rob Clark <robdclark@gmail.com> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- I suggest Alex picks up this patch for the EFI tree. --- lib/vsprintf.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)