Re: [PATCH v2 2/4] boot: fdt: Clean up env_get_bootm_size()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Wed, Mar 20, 2024 at 09:52:34PM +0100, Marek Vasut wrote:
> On 3/18/24 5:18 PM, Laurent Pinchart wrote:
> 
> >> @@ -142,7 +140,7 @@ phys_size_t env_get_bootm_size(void)
> >>   
> >>   	s = env_get("bootm_low");
> >>   	if (s)
> >> -		tmp = (phys_size_t)simple_strtoull(s, NULL, 16);
> >> +		tmp = simple_strtoull(s, NULL, 16);
> >>   	else
> >>   		tmp = start;
> >>   
> > 
> > Maybe you could even drop the tmp variable completely by writing this
> > 
> > 	if (s)
> > 		size -= simple_strtoull(s, NULL, 16) - start;
> > 
> > 	return size;
> > 
> > I've never liked variables named tmp :-)
> 
> No, let's not do this. With this FDT part, the code should be verbose 
> and as easy to understand at first glance as possible, no subtraction 
> assignments and other shenanigans please.

How about this ?

	s = env_get("bootm_low");
	if (s) {
		phys_addr_t low_addr = simple_strtoull(s, NULL, 16);
                size -= low_addr - start;
	}

	return size;

If you're going for readability, that's clearer than

	return size - (tmp - start);

and it also interestingly points out that tmp/low_addr should be a
phys_addr_t, not a phys_size_t.

-- 
Regards,

Laurent Pinchart