| Message ID | 20230420143339.2769414-2-yanaijie@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: libsas: remove empty branches and code simplification | expand |
On Thu, Apr 20, 2023 at 10:33:37PM +0800, Jason Yan wrote: > In sas_check_eeds() there is an empty branch. We can reverse the > test expression and then remove the empty branch. Also the the test > expression is a little bit complex so it deserves an individual > function. And make the continuing prototype lines indented after > the opening parenthesis to follow the standard coding style. > > Signed-off-by: Jason Yan <yanaijie@huawei.com> > --- > drivers/scsi/libsas/sas_expander.c | 39 +++++++++++++++--------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c > index dc670304f181..70fd4f439664 100644 > --- a/drivers/scsi/libsas/sas_expander.c > +++ b/drivers/scsi/libsas/sas_expander.c > @@ -1198,37 +1198,36 @@ static void sas_print_parent_topology_bug(struct domain_device *child, > sas_route_char(child, child_phy)); > } > > +static bool sas_eeds_valid(struct domain_device *parent, struct domain_device *child) Please avoid the overly long line.
On 2023/4/20 23:00, Christoph Hellwig wrote: > On Thu, Apr 20, 2023 at 10:33:37PM +0800, Jason Yan wrote: >> In sas_check_eeds() there is an empty branch. We can reverse the >> test expression and then remove the empty branch. Also the the test >> expression is a little bit complex so it deserves an individual >> function. And make the continuing prototype lines indented after >> the opening parenthesis to follow the standard coding style. >> >> Signed-off-by: Jason Yan <yanaijie@huawei.com> >> --- >> drivers/scsi/libsas/sas_expander.c | 39 +++++++++++++++--------------- >> 1 file changed, 19 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c >> index dc670304f181..70fd4f439664 100644 >> --- a/drivers/scsi/libsas/sas_expander.c >> +++ b/drivers/scsi/libsas/sas_expander.c >> @@ -1198,37 +1198,36 @@ static void sas_print_parent_topology_bug(struct domain_device *child, >> sas_route_char(child, child_phy)); >> } >> >> +static bool sas_eeds_valid(struct domain_device *parent, struct domain_device *child) > > Please avoid the overly long line. OK.
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index dc670304f181..70fd4f439664 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -1198,37 +1198,36 @@ static void sas_print_parent_topology_bug(struct domain_device *child, sas_route_char(child, child_phy)); } +static bool sas_eeds_valid(struct domain_device *parent, struct domain_device *child) +{ + struct sas_discovery *disc = &parent->port->disc; + + return (SAS_ADDR(disc->eeds_a) == SAS_ADDR(parent->sas_addr) || + SAS_ADDR(disc->eeds_a) == SAS_ADDR(child->sas_addr)) && + (SAS_ADDR(disc->eeds_b) == SAS_ADDR(parent->sas_addr) || + SAS_ADDR(disc->eeds_b) == SAS_ADDR(child->sas_addr)); +} + static int sas_check_eeds(struct domain_device *child, - struct ex_phy *parent_phy, - struct ex_phy *child_phy) + struct ex_phy *parent_phy, + struct ex_phy *child_phy) { int res = 0; struct domain_device *parent = child->parent; + struct sas_discovery *disc = &parent->port->disc; - if (SAS_ADDR(parent->port->disc.fanout_sas_addr) != 0) { + if (SAS_ADDR(disc->fanout_sas_addr) != 0) { res = -ENODEV; pr_warn("edge ex %016llx phy S:%02d <--> edge ex %016llx phy S:%02d, while there is a fanout ex %016llx\n", SAS_ADDR(parent->sas_addr), parent_phy->phy_id, SAS_ADDR(child->sas_addr), child_phy->phy_id, - SAS_ADDR(parent->port->disc.fanout_sas_addr)); - } else if (SAS_ADDR(parent->port->disc.eeds_a) == 0) { - memcpy(parent->port->disc.eeds_a, parent->sas_addr, - SAS_ADDR_SIZE); - memcpy(parent->port->disc.eeds_b, child->sas_addr, - SAS_ADDR_SIZE); - } else if (((SAS_ADDR(parent->port->disc.eeds_a) == - SAS_ADDR(parent->sas_addr)) || - (SAS_ADDR(parent->port->disc.eeds_a) == - SAS_ADDR(child->sas_addr))) - && - ((SAS_ADDR(parent->port->disc.eeds_b) == - SAS_ADDR(parent->sas_addr)) || - (SAS_ADDR(parent->port->disc.eeds_b) == - SAS_ADDR(child->sas_addr)))) - ; - else { + SAS_ADDR(disc->fanout_sas_addr)); + } else if (SAS_ADDR(disc->eeds_a) == 0) { + memcpy(disc->eeds_a, parent->sas_addr, SAS_ADDR_SIZE); + memcpy(disc->eeds_b, child->sas_addr, SAS_ADDR_SIZE); + } else if (!sas_eeds_valid(parent, child)) { res = -ENODEV; pr_warn("edge ex %016llx phy%02d <--> edge ex %016llx phy%02d link forms a third EEDS!\n", SAS_ADDR(parent->sas_addr),
In sas_check_eeds() there is an empty branch. We can reverse the test expression and then remove the empty branch. Also the the test expression is a little bit complex so it deserves an individual function. And make the continuing prototype lines indented after the opening parenthesis to follow the standard coding style. Signed-off-by: Jason Yan <yanaijie@huawei.com> --- drivers/scsi/libsas/sas_expander.c | 39 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 20 deletions(-)