diff mbox series

lib: utils: fdt_fixup: Fix compile error

Message ID 20230219063335.30952-1-peterlin@andestech.com
State Superseded
Headers show
Series lib: utils: fdt_fixup: Fix compile error | expand

Commit Message

Yu Chien Peter Lin Feb. 19, 2023, 6:33 a.m. UTC
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(-)

Comments

Andreas Schwab Feb. 19, 2023, 9:11 a.m. UTC | #1
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.
Xiang W Feb. 19, 2023, 9:28 a.m. UTC | #2
在 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
> 
>
Yu Chien Peter Lin Feb. 19, 2023, 5:44 p.m. UTC | #3
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@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
diff mbox series

Patch

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++) {