diff mbox series

tools: ftdgrep: correct the find regions loop in do_fdtgrep

Message ID 20200110085027.1.I4a80e4d2935f07164868f198fe868a0999be777e@changeid
State Superseded
Headers show
Series tools: ftdgrep: correct the find regions loop in do_fdtgrep | expand

Commit Message

Patrick Delaunay Jan. 10, 2020, 7:50 a.m. UTC
Update the loop executed in do_fdtgrep to find all the regions
and add a test for count > max_region only when the second passes
is executed.

This patch solve an issue if the number of region found (count)
is greater then the default value (max_region = count = 100):
the second pass is never executed, because the loop stops after
the first pass (i = 0, count > 100, max_regions = 100)
with error -1 and the error message
"Internal error with fdtgrep_find_region".

I also update the error message.

Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
---

 tools/fdtgrep.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Anatolij Gustschin Jan. 12, 2020, 7:51 p.m. UTC | #1
Hi Patrick,

On Fri, 10 Jan 2020 08:50:31 +0100
Patrick Delaunay patrick.delaunay at st.com wrote:
...
> diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
> index 8f44f599c1..bcf06b871d 100644
> --- a/tools/fdtgrep.c
> +++ b/tools/fdtgrep.c
> @@ -823,8 +823,10 @@ static int do_fdtgrep(struct display_info *disp, const char *filename)
>  		}
>  		if (count <= max_regions)
>  			break;
> +	}
> +	if (count > max_regions) {
>  		free(region);

This change moved free(region) out of the loop. If you do so, then
please also change malloc() in the loop to realloc():

	region = realloc(region, count * sizeof(struct fdt_region));

--
Anatolij
Patrick Delaunay Jan. 13, 2020, 8:24 a.m. UTC | #2
Hi,

> From: Anatolij Gustschin <agust at denx.de>
> Sent: dimanche 12 janvier 2020 20:51
> 
> Hi Patrick,
> 
> On Fri, 10 Jan 2020 08:50:31 +0100
> Patrick Delaunay patrick.delaunay at st.com wrote:
> ...
> > diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index
> > 8f44f599c1..bcf06b871d 100644
> > --- a/tools/fdtgrep.c
> > +++ b/tools/fdtgrep.c
> > @@ -823,8 +823,10 @@ static int do_fdtgrep(struct display_info *disp, const
> char *filename)
> >  		}
> >  		if (count <= max_regions)
> >  			break;
> > +	}
> > +	if (count > max_regions) {
> >  		free(region);
> 
> This change moved free(region) out of the loop. If you do so, then please also
> change malloc() in the loop to realloc():
> 
> 	region = realloc(region, count * sizeof(struct fdt_region));

Yes, I will push V2 with realloc to avoid the memory leak that I introduced..

Thanks for review 

> --
> Anatolij

Patrick
diff mbox series

Patch

diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
index 8f44f599c1..bcf06b871d 100644
--- a/tools/fdtgrep.c
+++ b/tools/fdtgrep.c
@@ -823,8 +823,10 @@  static int do_fdtgrep(struct display_info *disp, const char *filename)
 		}
 		if (count <= max_regions)
 			break;
+	}
+	if (count > max_regions) {
 		free(region);
-		fprintf(stderr, "Internal error with fdtgrep_find_region)(\n");
+		fprintf(stderr, "Internal error with fdtgrep_find_region()\n");
 		return -1;
 	}