On 3/20/24 10:00 PM, Laurent Pinchart wrote:
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);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);
I do not share this opinion, the subtraction assignment makes this harder to read. If that makes for a more verbose code, so be it.
and it also interestingly points out that tmp/low_addr should be a phys_addr_t, not a phys_size_t.
The original code already contains trivial 'tmp = start' assignment, so these two types should match, and they don't. Fixed in V4, thanks.