Re: [PATCH 1/3] scsi: libsas: Simplify sas_check_eeds()

[John Garry <john.g.garry@oracle.com>]

On 03/04/2023 02:37, Jason Yan wrote:
>>>
>>> diff --git a/drivers/scsi/libsas/sas_expander.c 
>>> b/drivers/scsi/libsas/sas_expander.c
>>> index dc670304f181..048a931d856a 100644
>>> --- a/drivers/scsi/libsas/sas_expander.c
>>> +++ b/drivers/scsi/libsas/sas_expander.c
>>> @@ -1198,37 +1198,35 @@ 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;
>>
>> Missing blank line after declaration.
> 
> OK.
> 
>>
>>> +    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))));
>>
>> Drop the inner-most and outter-most parenthesis.
> 
> No problem.

Personally I think that the flow:

if (SAS_ADDR(disc->eeds_a) == SAS_ADDR(parent->sas_addr))
	return true;
if (...)
	return true;
if (...)
	return true;
return false;

..reads a bit better (than this and the current code). However I don't 
feel too strongly about it.

Thanks,
John