[PATCH 1/1] pseries/hotplug-memory.c: Replace nested ifs by switch-case
Subsystems:
linux for powerpc (32-bit and 64-bit) , the rest
STALE2567d
LANDED
Landed in mainline as 9616dda1aadd on 2019-08-05.
5 messages,
4 authors,
2019-08-10 · open the first message on its own page
I noticed these nested ifs can be easily replaced by switch-cases,
which can improve readability.
Signed-off-by: Leonardo Bras <redacted>
---
.../platforms/pseries/hotplug-memory.c | 26 +++++++++++++------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 46d0d35b9ca4..8e700390f3d6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -880,34 +880,44 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
switch ( hp_elog -> action ) {
case PSERIES_HP_ELOG_ACTION_ADD :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_add_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_add_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_add_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ;
case PSERIES_HP_ELOG_ACTION_REMOVE :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_remove_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_remove_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_remove_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ; --
2.20.1
On 02.08.19 00:52, Leonardo Bras wrote: I noticed these nested ifs can be easily replaced by switch-cases,
which can improve readability.
Signed-off-by: Leonardo Bras <redacted>
---
.../platforms/pseries/hotplug-memory.c | 26 +++++++++++++------
1 file changed, 18 insertions(+), 8 deletions(-)
More LOC but seems to be the right thing to do
Reviewed-by: David Hildenbrand <redacted>
quoted hunk diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 46d0d35b9ca4..8e700390f3d6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -880,34 +880,44 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
switch ( hp_elog -> action ) {
case PSERIES_HP_ELOG_ACTION_ADD :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_add_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_add_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_add_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ;
case PSERIES_HP_ELOG_ACTION_REMOVE :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_remove_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_remove_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_remove_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ;
--
Thanks,
David / dhildenb
Leonardo Bras [off-list ref] writes: I noticed these nested ifs can be easily replaced by switch-cases,
which can improve readability.
Signed-off-by: Leonardo Bras <redacted>
---
.../platforms/pseries/hotplug-memory.c | 26 +++++++++++++------
1 file changed, 18 insertions(+), 8 deletions(-)
Thanks, this looks sensible.
Please use "powerpc/" as the prefix on your patches, eg. in this case:
"powerpc/pseries/hotplug-memory.c: Replace nested ifs by switch-case"
I'll fix it up this time when I apply.
cheers
quoted hunk diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 46d0d35b9ca4..8e700390f3d6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -880,34 +880,44 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
switch ( hp_elog -> action ) {
case PSERIES_HP_ELOG_ACTION_ADD :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_add_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_add_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_add_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ;
case PSERIES_HP_ELOG_ACTION_REMOVE :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_remove_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_remove_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_remove_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ; --
2.20.1
On Fri, 2019-08-02 at 22:26 +1000, Michael Ellerman wrote: Leonardo Bras [off-list ref] writes: quoted I noticed these nested ifs can be easily replaced by switch-cases,
which can improve readability.
Signed-off-by: Leonardo Bras <redacted>
---
.../platforms/pseries/hotplug-memory.c | 26 +++++++++++++------
1 file changed, 18 insertions(+), 8 deletions(-)
Thanks, this looks sensible.
Please use "powerpc/" as the prefix on your patches, eg. in this case:
"powerpc/pseries/hotplug-memory.c: Replace nested ifs by switch-case" Ok, I will make sure to do that next time.
Thanks!
I'll fix it up this time when I apply.
cheers
quoted diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 46d0d35b9ca4..8e700390f3d6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -880,34 +880,44 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
switch ( hp_elog -> action ) {
case PSERIES_HP_ELOG_ACTION_ADD :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_add_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_add_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_add_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ;
case PSERIES_HP_ELOG_ACTION_REMOVE :
- if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_COUNT ) {
+ switch ( hp_elog -> id_type ) {
+ case PSERIES_HP_ELOG_ID_DRC_COUNT :
count = hp_elog -> _drc_u . drc_count ;
rc = dlpar_memory_remove_by_count ( count );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_INDEX ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_INDEX :
drc_index = hp_elog -> _drc_u . drc_index ;
rc = dlpar_memory_remove_by_index ( drc_index );
- } else if ( hp_elog -> id_type == PSERIES_HP_ELOG_ID_DRC_IC ) {
+ break ;
+ case PSERIES_HP_ELOG_ID_DRC_IC :
count = hp_elog -> _drc_u . ic . count ;
drc_index = hp_elog -> _drc_u . ic . index ;
rc = dlpar_memory_remove_by_ic ( count , drc_index );
- } else {
+ break ;
+ default :
rc = - EINVAL ;
+ break ;
}
break ; --
2.20.1
On Thu, 2019-08-01 at 22:52:51 UTC, Leonardo Bras wrote: I noticed these nested ifs can be easily replaced by switch-cases,
which can improve readability.
Signed-off-by: Leonardo Bras <redacted>
Reviewed-by: David Hildenbrand <redacted>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/9616dda1aaddb2080122aaeab96ad7fc064e36b4
cheers