[U-Boot] [PATCH] efi_loader: add missing breaks

[U-Boot] [PATCH] efi_loader: add missing breaks

[Rob Clark]

Otherwise with GUID partition types you would end up with things like:

  .../HD(Part0,Sig6252c819-4624-4995-8d16-abc9cd5d4130)/HD(Part0,MBRType=02,SigType=02)

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
Reported by 'ykaukab' on IRC.

Not sure if someone already sent a similar patch.

 lib/efi_loader/efi_device_path_to_text.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index 62771338f0..3b703301ff 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -135,10 +135,12 @@ static char *dp_media(char *s, struct efi_device_path *dp)
 		case SIG_TYPE_GUID:
 			s += sprintf(s, "/HD(Part%d,Sig%pUl)",
 				     hddp->partition_number, sig);
+			break;
 		default:
 			s += sprintf(s, "/HD(Part%d,MBRType=%02x,SigType=%02x)",
 				     hddp->partition_number, hddp->partmap_type,
 				     hddp->signature_type);
+			break;
 		}
 
 		break;
-- 
2.13.6

[U-Boot] [PATCH] efi_loader: add missing breaks

[Heinrich Schuchardt]

On 11/30/2017 03:02 PM, Rob Clark wrote:
> Otherwise with GUID partition types you would end up with things like:
> 
>    .../HD(Part0,Sig6252c819-4624-4995-8d16-abc9cd5d4130)/HD(Part0,MBRType=02,SigType=02)
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> Reported by 'ykaukab' on IRC.
> 
> Not sure if someone already sent a similar patch.
> 
>   lib/efi_loader/efi_device_path_to_text.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> index 62771338f0..3b703301ff 100644
> --- a/lib/efi_loader/efi_device_path_to_text.c
> +++ b/lib/efi_loader/efi_device_path_to_text.c
> @@ -135,10 +135,12 @@ static char *dp_media(char *s, struct efi_device_path *dp)
>   		case SIG_TYPE_GUID:
>   			s += sprintf(s, "/HD(Part%d,Sig%pUl)",

This is not the format defined in UEFI Spec 2.7:

HD(Partition, Type, Signature, Start, Size)
HD(Partition, Type, Signature) (Display only)

We should output something like:
/HD(1,MBR,0xA0021243,0x800,0x2EE00)

>   				     hddp->partition_number, sig);
> +			break;
>   		default:
>   			s += sprintf(s, "/HD(Part%d,MBRType=%02x,SigType=%02x)",
>   				     hddp->partition_number, hddp->partmap_type,
>   				     hddp->signature_type);

See above.

> +			break;

This line is superfluous at the end of a switch block.

Could you, please, rebase your patch on
[PATCH v3 04/18] efi_loader: fix efi_convert_device_node_to_text
https://lists.denx.de/pipermail/u-boot/2017-November/312523.html

Best regards

Heinrich

>   		}
>   
>   		break;
> 

[U-Boot] [PATCH] efi_loader: add missing breaks

[Alexander Graf]

On 30.11.17 15:21, Heinrich Schuchardt wrote:
> 
> 
> On 11/30/2017 03:02 PM, Rob Clark wrote:
>> Otherwise with GUID partition types you would end up with things like:
>>
>>   
>> .../HD(Part0,Sig6252c819-4624-4995-8d16-abc9cd5d4130)/HD(Part0,MBRType=02,SigType=02)
>>
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> Reported by 'ykaukab' on IRC.
>>
>> Not sure if someone already sent a similar patch.
>>
>>   lib/efi_loader/efi_device_path_to_text.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/efi_loader/efi_device_path_to_text.c
>> b/lib/efi_loader/efi_device_path_to_text.c
>> index 62771338f0..3b703301ff 100644
>> --- a/lib/efi_loader/efi_device_path_to_text.c
>> +++ b/lib/efi_loader/efi_device_path_to_text.c
>> @@ -135,10 +135,12 @@ static char *dp_media(char *s, struct
>> efi_device_path *dp)
>>           case SIG_TYPE_GUID:
>>               s += sprintf(s, "/HD(Part%d,Sig%pUl)",
> 
> This is not the format defined in UEFI Spec 2.7:
> 
> HD(Partition, Type, Signature, Start, Size)
> HD(Partition, Type, Signature) (Display only)
> 
> We should output something like:
> /HD(1,MBR,0xA0021243,0x800,0x2EE00)
> 
>>                        hddp->partition_number, sig);
>> +            break;
>>           default:
>>               s += sprintf(s, "/HD(Part%d,MBRType=%02x,SigType=%02x)",
>>                        hddp->partition_number, hddp->partmap_type,
>>                        hddp->signature_type);
> 
> See above.
> 
>> +            break;
> 
> This line is superfluous at the end of a switch block.

I actually like the symmetry of a break at the end of default (or
whatever the last switch case is).

> 
> Could you, please, rebase your patch on
> [PATCH v3 04/18] efi_loader: fix efi_convert_device_node_to_text
> https://lists.denx.de/pipermail/u-boot/2017-November/312523.html

I've just manually fixed it up and applied it now.


Alex

[U-Boot] efi_loader: add missing breaks

[Alexander Graf]

> Otherwise with GUID partition types you would end up with things like:
> 
>   .../HD(Part0,Sig6252c819-4624-4995-8d16-abc9cd5d4130)/HD(Part0,MBRType=02,SigType=02)
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Thanks, applied to efi-next

Alex