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);
        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);

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.

Reply via email to