| Message ID | aa72e3371fa4ab4806cd866c569718d766d3142e.1690985045.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: address MISRA C:2012 Rule 2.1 | expand |
On Wed, 2 Aug 2023, Nicola Vetrini wrote: > The variable declaration is moved where it's actually used, rather > than being declared in the switch before any clause, thus being > classified as unreachable code. > > No functional changes. > > Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> > --- > xen/arch/x86/efi/efi-boot.h | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h > index 92f4cfe8bd..b00441b1a2 100644 > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -390,8 +390,6 @@ static void __init efi_arch_edd(void) > { > switch ( DevicePathType(devp.DevPath) ) > { > - const u8 *p; > - > case ACPI_DEVICE_PATH: > if ( state != root || boot_edd_info_nr > EDD_INFO_MAX ) > break; > @@ -463,7 +461,8 @@ static void __init efi_arch_edd(void) > params->device_path_info_length = > sizeof(struct edd_device_params) - > offsetof(struct edd_device_params, key); > - for ( p = (const u8 *)¶ms->key; p < ¶ms->checksum; ++p ) > + for ( const u8 *p = (const u8 *)¶ms->key; > + p < ¶ms->checksum; ++p ) In Xen we don't mix declaration and code. So the following is not something we use: for (int i = 0; i < 10; i++) I think you'd have to introduce another block under case MESSAGING_DEVICE_PATH so that you can moved const u8 *p there > params->checksum -= *p; > break; > case MEDIA_DEVICE_PATH: > -- > 2.34.1 >
On 03.08.2023 04:08, Stefano Stabellini wrote: > On Wed, 2 Aug 2023, Nicola Vetrini wrote: >> The variable declaration is moved where it's actually used, rather >> than being declared in the switch before any clause, thus being >> classified as unreachable code. >> >> No functional changes. >> >> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >> --- >> xen/arch/x86/efi/efi-boot.h | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h >> index 92f4cfe8bd..b00441b1a2 100644 >> --- a/xen/arch/x86/efi/efi-boot.h >> +++ b/xen/arch/x86/efi/efi-boot.h >> @@ -390,8 +390,6 @@ static void __init efi_arch_edd(void) >> { >> switch ( DevicePathType(devp.DevPath) ) >> { >> - const u8 *p; >> - >> case ACPI_DEVICE_PATH: >> if ( state != root || boot_edd_info_nr > EDD_INFO_MAX ) >> break; >> @@ -463,7 +461,8 @@ static void __init efi_arch_edd(void) >> params->device_path_info_length = >> sizeof(struct edd_device_params) - >> offsetof(struct edd_device_params, key); >> - for ( p = (const u8 *)¶ms->key; p < ¶ms->checksum; ++p ) >> + for ( const u8 *p = (const u8 *)¶ms->key; >> + p < ¶ms->checksum; ++p ) > > In Xen we don't mix declaration and code. So the following is not > something we use: > > for (int i = 0; i < 10; i++) You're aware that we gained a couple of such uses already? I also think that when we discussed this we said this style could be at least okay-ish (until formalized in ./CODING_STYLE). What I'm unhappy with here is the retaining of u8, when it could easily become uint8_t at this occasion. Jan
On 03/08/2023 10:57, Jan Beulich wrote: > On 03.08.2023 04:08, Stefano Stabellini wrote: >> On Wed, 2 Aug 2023, Nicola Vetrini wrote: >>> The variable declaration is moved where it's actually used, rather >>> than being declared in the switch before any clause, thus being >>> classified as unreachable code. >>> >>> No functional changes. >>> >>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> >>> --- >>> xen/arch/x86/efi/efi-boot.h | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/x86/efi/efi-boot.h >>> b/xen/arch/x86/efi/efi-boot.h >>> index 92f4cfe8bd..b00441b1a2 100644 >>> --- a/xen/arch/x86/efi/efi-boot.h >>> +++ b/xen/arch/x86/efi/efi-boot.h >>> @@ -390,8 +390,6 @@ static void __init efi_arch_edd(void) >>> { >>> switch ( DevicePathType(devp.DevPath) ) >>> { >>> - const u8 *p; >>> - >>> case ACPI_DEVICE_PATH: >>> if ( state != root || boot_edd_info_nr > >>> EDD_INFO_MAX ) >>> break; >>> @@ -463,7 +461,8 @@ static void __init efi_arch_edd(void) >>> params->device_path_info_length = >>> sizeof(struct edd_device_params) - >>> offsetof(struct edd_device_params, key); >>> - for ( p = (const u8 *)¶ms->key; p < >>> ¶ms->checksum; ++p ) >>> + for ( const u8 *p = (const u8 *)¶ms->key; >>> + p < ¶ms->checksum; ++p ) >> >> In Xen we don't mix declaration and code. So the following is not >> something we use: >> >> for (int i = 0; i < 10; i++) > > You're aware that we gained a couple of such uses already? I also think > that when we discussed this we said this style could be at least > okay-ish (until formalized in ./CODING_STYLE). > > What I'm unhappy with here is the retaining of u8, when it could easily > become uint8_t at this occasion. > > Jan Sure
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 92f4cfe8bd..b00441b1a2 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -390,8 +390,6 @@ static void __init efi_arch_edd(void) { switch ( DevicePathType(devp.DevPath) ) { - const u8 *p; - case ACPI_DEVICE_PATH: if ( state != root || boot_edd_info_nr > EDD_INFO_MAX ) break; @@ -463,7 +461,8 @@ static void __init efi_arch_edd(void) params->device_path_info_length = sizeof(struct edd_device_params) - offsetof(struct edd_device_params, key); - for ( p = (const u8 *)¶ms->key; p < ¶ms->checksum; ++p ) + for ( const u8 *p = (const u8 *)¶ms->key; + p < ¶ms->checksum; ++p ) params->checksum -= *p; break; case MEDIA_DEVICE_PATH:
The variable declaration is moved where it's actually used, rather than being declared in the switch before any clause, thus being classified as unreachable code. No functional changes. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/arch/x86/efi/efi-boot.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)