Re: [PATCH v2 1/5] soc: qcom: smem: Use le32_to_cpu for partition size comparison

[Bjorn Andersson]

On Fri 15 Sep 13:53 PDT 2017, Chris Lew wrote:

> 
> 
> On 9/15/2017 11:39 AM, Stephen Boyd wrote:
> > On 09/14, Chris Lew wrote:
> > > Endianness can vary in the system, add le32_to_cpu when comparing
> > > size values from smem.
> > > 
> > > Signed-off-by: Chris Lew <clew@xxxxxxxxxxxxxx>
> > > ---
> > > 
> > > Changes since v1:
> > > - New change
> > > 
> > >   drivers/soc/qcom/smem.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> > > index c28275be0038..db04c45d4132 100644
> > > --- a/drivers/soc/qcom/smem.c
> > > +++ b/drivers/soc/qcom/smem.c
> > > @@ -698,7 +698,7 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
> > >   			return -EINVAL;
> > >   		}
> > > -		if (header->size != entry->size) {
> > > +		if (le32_to_cpu(header->size) != le32_to_cpu(entry->size)) {
> > 
> > Also, it doesn't really matter. We're comparing two numbers with
> > the same endianness, so comparing them for equality before or
> > after swapping makes no difference. Sparse also (correctly)
> > doesn't complain here, because adding the conversion is not
> > necessary. Drop this patch?
> > 
> 
> Hey Bjorn, should we remove this patch? You had flagged this comparison in
> the first version of the global partition changes.
> 

As Stephen says; it's (technically) unnecessary.

But I think we should take this patch anyways, for the sake of saving
future readers of having to think about why there's one case in the
entire file where we don't do this translation.

Regards,
Bjorn