[PATCH] lib: utils: fdt_fixup: Fix compile error

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Yu Chien Peter Lin]

When building with GCC-10 or older versions, it throws the following
error:

 CC-DEP    platform/generic/lib/utils/fdt/fdt_fixup.dep
 CC        platform/generic/lib/utils/fdt/fdt_fixup.o
lib/utils/fdt/fdt_fixup.c: In function 'fdt_reserved_memory_fixup':
lib/utils/fdt/fdt_fixup.c:376:2: error: label at end of compound statement
  376 |  next_entry:
      |  ^~~~~~~~~~

Thus move the label "next_entry" to the beginning of the for loop.
Fixed issue#288 [1]

[1]: https://github.com/riscv-software-src/opensbi/issues/288

Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
---
 lib/utils/fdt/fdt_fixup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c
index 619e4f5..9dac98a 100644
--- a/lib/utils/fdt/fdt_fixup.c
+++ b/lib/utils/fdt/fdt_fixup.c
@@ -345,6 +345,7 @@ int fdt_reserved_memory_fixup(void *fdt)
 
 	i = 0;
 	sbi_domain_for_each_memregion(dom, reg) {
+next_entry:
 		/* Ignore MMIO or READABLE or WRITABLE or EXECUTABLE regions */
 		if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
 			continue;
@@ -373,7 +374,6 @@ int fdt_reserved_memory_fixup(void *fdt)
 		filtered_base[i] = reg->base;
 		filtered_order[i] = reg->order;
 		i++;
-	next_entry:
 	}
 
 	for (j = 0; j < i; j++) {
-- 
2.34.1

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Andreas Schwab]

On Feb 19 2023, Yu Chien Peter Lin wrote:

> Thus move the label "next_entry" to the beginning of the for loop.

This changes the semantics, because jumping to the top of the loop skips
the update expression.

-- 
Andreas Schwab, schwab at linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Xiang W]

? 2023-02-19???? 14:33 +0800?Yu Chien Peter Lin???
> When building with GCC-10 or older versions, it throws the following
> error:
> 
> ?CC-DEP??? platform/generic/lib/utils/fdt/fdt_fixup.dep
> ?CC??????? platform/generic/lib/utils/fdt/fdt_fixup.o
> lib/utils/fdt/fdt_fixup.c: In function 'fdt_reserved_memory_fixup':
> lib/utils/fdt/fdt_fixup.c:376:2: error: label at end of compound statement
> ? 376 |? next_entry:
> ????? |? ^~~~~~~~~~
> 
> Thus move the label "next_entry" to the beginning of the for loop.
> Fixed issue#288 [1]
> 
> [1]: https://github.com/riscv-software-src/opensbi/issues/288
> 
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> ---
> ?lib/utils/fdt/fdt_fixup.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c
> index 619e4f5..9dac98a 100644
> --- a/lib/utils/fdt/fdt_fixup.c
> +++ b/lib/utils/fdt/fdt_fixup.c
> @@ -345,6 +345,7 @@ int fdt_reserved_memory_fixup(void *fdt)
> ?
> ????????i = 0;
> ????????sbi_domain_for_each_memregion(dom, reg) {
> +next_entry:
> ????????????????/* Ignore MMIO or READABLE or WRITABLE or EXECUTABLE regions */
> ????????????????if (reg->flags & SBI_DOMAIN_MEMREGION_MMIO)
> ????????????????????????continue;
> @@ -373,7 +374,6 @@ int fdt_reserved_memory_fixup(void *fdt)
> ????????????????filtered_base[i] = reg->base;
> ????????????????filtered_order[i] = reg->order;
> ????????????????i++;
> -???????next_entry:
> ????????}
I suggest to remove goto. 

diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c
index 619e4f5..e23517d 100644
--- a/lib/utils/fdt/fdt_fixup.c
+++ b/lib/utils/fdt/fdt_fixup.c
@@ -361,19 +361,22 @@ int fdt_reserved_memory_fixup(void *fdt)
                        return SBI_ENOSPC;
                }
 
+               int overlap = 0;
                addr = reg->base;
                for (j = 0; j < i; j++) {
                        if (addr == filtered_base[j]
                            && filtered_order[j] < reg->order) {
+                               overlap = 1;
                                filtered_order[j] = reg->order;
-                               goto next_entry;
+                               break;
                        }
                }
 
-               filtered_base[i] = reg->base;
-               filtered_order[i] = reg->order;
-               i++;
-       next_entry:
+               if (!overlap) {
+                       filtered_base[i] = reg->base;
+                       filtered_order[i] = reg->order;
+                       i++;
+               }
        }
 
        for (j = 0; j < i; j++) {



Regards,
Xiang
> ?
> ????????for (j = 0; j < i; j++) {
> -- 
> 2.34.1
> 
> 

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Xiang W]

When building with GCC-10 or older versions, it throws the following
error:

 CC-DEP    platform/generic/lib/utils/fdt/fdt_fixup.dep
 CC        platform/generic/lib/utils/fdt/fdt_fixup.o
lib/utils/fdt/fdt_fixup.c: In function 'fdt_reserved_memory_fixup':
lib/utils/fdt/fdt_fixup.c:376:2: error: label at end of compound statement
  376 |  next_entry:
      |  ^~~~~~~~~~

Thus move the label "next_entry" to the beginning of the for loop.
Fixed issue#288 [1]

[1]: https://github.com/riscv-software-src/opensbi/issues/288

Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
Signed-off-by: Xiang W <wxjstz@126.com>
---
 lib/utils/fdt/fdt_fixup.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c
index 619e4f5..e23517d 100644
--- a/lib/utils/fdt/fdt_fixup.c
+++ b/lib/utils/fdt/fdt_fixup.c
@@ -361,19 +361,22 @@ int fdt_reserved_memory_fixup(void *fdt)
 			return SBI_ENOSPC;
 		}
 
+		int overlap = 0;
 		addr = reg->base;
 		for (j = 0; j < i; j++) {
 			if (addr == filtered_base[j]
 			    && filtered_order[j] < reg->order) {
+				overlap = 1;
 				filtered_order[j] = reg->order;
-				goto next_entry;
+				break;
 			}
 		}
 
-		filtered_base[i] = reg->base;
-		filtered_order[i] = reg->order;
-		i++;
-	next_entry:
+		if (!overlap) {
+			filtered_base[i] = reg->base;
+			filtered_order[i] = reg->order;
+			i++;
+		}
 	}
 
 	for (j = 0; j < i; j++) {
-- 
2.39.1

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Andreas Schwab]

On Feb 19 2023, Xiang W wrote:

> Thus move the label "next_entry" to the beginning of the for loop.

This is not what the patch does.

-- 
Andreas Schwab, schwab at linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Xiang W]

? 2023-02-19???? 11:20 +0100?Andreas Schwab???
> On Feb 19 2023, Xiang W wrote:
> 
> > Thus move the label "next_entry" to the beginning of the for loop.
> 
> This is not what the patch does.
new patch at https://lists.infradead.org/pipermail/opensbi/2023-February/004494.html

Regards,
Xiang
> 

[PATCH] lib: utils: fdt_fixup: Fix compile error

[Yu-Chien Peter Lin]

On Sun, Feb 19, 2023 at 10:11:09AM +0100, Andreas Schwab wrote:
> On Feb 19 2023, Yu Chien Peter Lin wrote:
> 
> > Thus move the label "next_entry" to the beginning of the for loop.
> 
> This changes the semantics, because jumping to the top of the loop skips
> the update expression.
> 
> -- 
> Andreas Schwab, schwab at linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."

Hi Andreas,

Thanks for pointing this out!

Hi Xiang,

I've set the patch state to superseded,
Could you send your fix?

Thanks
Peter Lin