[PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

buildargv() and freeargv() are pairs, so need be sure of them always
paired to avoid memory leak.

2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>

	* common/sim-options.c (sim_args_command): Call freeargv() when
	failure occurs.
	* mcore/interp.c (sim_do_command): Call freeargv() before return.
	* microblaze/interp.c (sim_do_command): Call freeargv() before
	return.
---
 sim/ChangeLog            | 8 ++++++++
 sim/common/sim-options.c | 5 ++++-
 sim/mcore/interp.c       | 3 +++
 sim/microblaze/interp.c  | 3 +++
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/sim/ChangeLog b/sim/ChangeLog
index 03c244b..a5c3f5d 100644
--- a/sim/ChangeLog
+++ b/sim/ChangeLog
@@ -1,3 +1,11 @@
+2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
+
+	* common/sim-options.c (sim_args_command): Call freeargv() when
+	failure occurs.
+	* mcore/interp.c (sim_do_command): Call freeargv() before return.
+	* microblaze/interp.c (sim_do_command): Call freeargv() before
+	return.
+
 2014-07-01  Chen Gang <gang.chen.5i5j@gmail.com>
 
 	* sim/microblaze/interp.c: Use long int format instead of int
diff --git a/sim/common/sim-options.c b/sim/common/sim-options.c
index c49220e..814edcf 100644
--- a/sim/common/sim-options.c
+++ b/sim/common/sim-options.c
@@ -993,7 +993,10 @@ sim_args_command (SIM_DESC sd, const char *cmd)
       sim_cpu *cpu;
 
       if (argv [0] == NULL)
-	return SIM_RC_OK; /* FIXME - perhaps help would be better */
+	{
+	  freeargv (argv);
+	  return SIM_RC_OK; /* FIXME - perhaps help would be better */
+	}
 
       /* First check for a cpu selector.  */
       {
diff --git a/sim/mcore/interp.c b/sim/mcore/interp.c
index d2edd12..dfaa6aa 100644
--- a/sim/mcore/interp.c
+++ b/sim/mcore/interp.c
@@ -2143,6 +2143,7 @@ sim_do_command (sd, cmd)
 	  if ((simargv[1] == NULL) || (simargv[2] == NULL))
 	    {
 	      fprintf (stderr, "Error: missing argument to watch cmd.\n");
+	      freeargv (simargv);
 	      return;
 	    }
 	  
@@ -2187,6 +2188,8 @@ sim_do_command (sd, cmd)
 	  fprintf (stderr,"Error: \"%s\" is not a valid M.CORE simulator command.\n",
 		   cmd);
 	}
+
+      freeargv (simargv);
     }
   else
     {
diff --git a/sim/microblaze/interp.c b/sim/microblaze/interp.c
index 1c8a22d..4fc4595 100644
--- a/sim/microblaze/interp.c
+++ b/sim/microblaze/interp.c
@@ -1019,6 +1019,7 @@ sim_do_command (SIM_DESC sd, const char *cmd)
 	  if ((simargv[1] == NULL) || (simargv[2] == NULL))
 	    {
 	      fprintf (stderr, "Error: missing argument to watch cmd.\n");
+	      freeargv (simargv);
 	      return;
 	    }
 
@@ -1062,6 +1063,8 @@ sim_do_command (SIM_DESC sd, const char *cmd)
 	  fprintf (stderr,"Error: \"%s\" is not a valid M.CORE simulator command.\n",
 		   cmd);
 	}
+
+      freeargv (simargv);
     }
   else
     {
-- 
1.9.3 (Apple Git-50)

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Michael Eager]

On 01/28/15 03:45, Chen Gang S wrote:
> buildargv() and freeargv() are pairs, so need be sure of them always
> paired to avoid memory leak.
>
> 2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
>
> 	* common/sim-options.c (sim_args_command): Call freeargv() when
> 	failure occurs.
> 	* mcore/interp.c (sim_do_command): Call freeargv() before return.
> 	* microblaze/interp.c (sim_do_command): Call freeargv() before
> 	return.


OK for Microblaze.

There appear to be other places where buildargv() is not followed by
freeargv().  See sim/common/run.c.  There may be others.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 1/28/15 23:53, Michael Eager wrote:
> On 01/28/15 03:45, Chen Gang S wrote:
>> buildargv() and freeargv() are pairs, so need be sure of them always
>> paired to avoid memory leak.
>>
>> 2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
>>
>>     * common/sim-options.c (sim_args_command): Call freeargv() when
>>     failure occurs.
>>     * mcore/interp.c (sim_do_command): Call freeargv() before return.
>>     * microblaze/interp.c (sim_do_command): Call freeargv() before
>>     return.
> 
> 
> OK for Microblaze.
> 
> There appear to be other places where buildargv() is not followed by
> freeargv().  See sim/common/run.c.  There may be others.
> 

For me, I intended to skip buildargv() in "sim/common/run.c", because it
may contents read only memory. It is in main(), also main() often uses
exit(), so I skip it, it doesn't matter.

gdb also uses buildargv(), but it just wraps buildargv() and freeargv(),
so at least it is another patch (either, at least now, I do not find any
issues about it in gdb).

Except the above 2, for me, there are no any other places to use
buildargv().


Does the patch comments need to mention about the contents above?

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Michael Eager]

On 01/28/15 14:19, Chen Gang S wrote:
> On 1/28/15 23:53, Michael Eager wrote:
>> On 01/28/15 03:45, Chen Gang S wrote:
>>> buildargv() and freeargv() are pairs, so need be sure of them always
>>> paired to avoid memory leak.
>>>
>>> 2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
>>>
>>>      * common/sim-options.c (sim_args_command): Call freeargv() when
>>>      failure occurs.
>>>      * mcore/interp.c (sim_do_command): Call freeargv() before return.
>>>      * microblaze/interp.c (sim_do_command): Call freeargv() before
>>>      return.
>>
>>
>> OK for Microblaze.
>>
>> There appear to be other places where buildargv() is not followed by
>> freeargv().  See sim/common/run.c.  There may be others.
>>
>
> For me, I intended to skip buildargv() in "sim/common/run.c", because it
> may contents read only memory. It is in main(), also main() often uses
> exit(), so I skip it, it doesn't matter.
>
> gdb also uses buildargv(), but it just wraps buildargv() and freeargv(),
> so at least it is another patch (either, at least now, I do not find any
> issues about it in gdb).
>
> Except the above 2, for me, there are no any other places to use
> buildargv().
>
>
> Does the patch comments need to mention about the contents above?

No, you don't need to mention what is not modified.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 1/29/15 08:04, Michael Eager wrote:
> On 01/28/15 14:19, Chen Gang S wrote:
>> On 1/28/15 23:53, Michael Eager wrote:
>>> On 01/28/15 03:45, Chen Gang S wrote:
>>>> buildargv() and freeargv() are pairs, so need be sure of them always
>>>> paired to avoid memory leak.
>>>>
>>>> 2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
>>>>
>>>>      * common/sim-options.c (sim_args_command): Call freeargv() when
>>>>      failure occurs.
>>>>      * mcore/interp.c (sim_do_command): Call freeargv() before return.
>>>>      * microblaze/interp.c (sim_do_command): Call freeargv() before
>>>>      return.
>>>
>>>
>>> OK for Microblaze.
>>>
>>> There appear to be other places where buildargv() is not followed by
>>> freeargv().  See sim/common/run.c.  There may be others.
>>>
>>
>> For me, I intended to skip buildargv() in "sim/common/run.c", because it
>> may contents read only memory. It is in main(), also main() often uses
>> exit(), so I skip it, it doesn't matter.
>>
>> gdb also uses buildargv(), but it just wraps buildargv() and freeargv(),
>> so at least it is another patch (either, at least now, I do not find any
>> issues about it in gdb).
>>
>> Except the above 2, for me, there are no any other places to use
>> buildargv().
>>

Oh, sorry, the 3rd sentence is incorrect, it should be:

  "Except the above 2, for me, there are no any other places to cause
   buildargv() related issues or doubts".

>>
>> Does the patch comments need to mention about the contents above?
> 
> No, you don't need to mention what is not modified.
> 
> 

OK, thanks.

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On 29 Jan 2015 06:19, Chen Gang S wrote:
> On 1/28/15 23:53, Michael Eager wrote:
> > On 01/28/15 03:45, Chen Gang S wrote:
> >> buildargv() and freeargv() are pairs, so need be sure of them always
> >> paired to avoid memory leak.
> > 
> > There appear to be other places where buildargv() is not followed by
> > freeargv().  See sim/common/run.c.  There may be others.
> 
> For me, I intended to skip buildargv() in "sim/common/run.c", because it
> may contents read only memory. It is in main(), also main() often uses
> exit(), so I skip it, it doesn't matter.

i'm not too worried about run.c.  in fact, the more it bitrots the better ... it 
gets people to switch to nrun.c :).
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Joel Brobecker]

[binutils does not need to be copied in this case, as you're only
touching files in the simulator, which is part of the GDB project]

> 2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
> 
> 	* common/sim-options.c (sim_args_command): Call freeargv() when
> 	failure occurs.
> 	* mcore/interp.c (sim_do_command): Call freeargv() before return.
> 	* microblaze/interp.c (sim_do_command): Call freeargv() before
> 	return.

Small procedural request, Chen. Those 3 changes are pretty much
independent, so it's highly preferable to submit them separately.
This has a number of advantages: We can review each one of them
individually, with possibly different reviewers, and that makes
tracking of which part has been reviewed a lot easier. Also, by
having them submitted separately, you can have one patch per piece,
which means that if one patch turns out to be incorrect, we can
easily revert just that patch using git, rather than doing a semi-
revert by hand.

-- 
Joel

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 1/29/15 12:49, Joel Brobecker wrote:
> [binutils does not need to be copied in this case, as you're only
> touching files in the simulator, which is part of the GDB project]
> 
>> 2015-01-28  Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> 	* common/sim-options.c (sim_args_command): Call freeargv() when
>> 	failure occurs.
>> 	* mcore/interp.c (sim_do_command): Call freeargv() before return.
>> 	* microblaze/interp.c (sim_do_command): Call freeargv() before
>> 	return.
> 
> Small procedural request, Chen. Those 3 changes are pretty much
> independent, so it's highly preferable to submit them separately.
> This has a number of advantages: We can review each one of them
> individually, with possibly different reviewers, and that makes
> tracking of which part has been reviewed a lot easier. Also, by
> having them submitted separately, you can have one patch per piece,
> which means that if one patch turns out to be incorrect, we can
> easily revert just that patch using git, rather than doing a semi-
> revert by hand.
> 

OK, thanks. I shall send 3 separated patches for it within this month.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

[PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

Or there will be memory leak.

2015-01-31  Chen Gang <gang.chen.5i5j@gmail.com>

	* mcore/interp.c (sim_do_command): Call freeargv() before return.
---
 sim/ChangeLog      | 4 ++++
 sim/mcore/interp.c | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/sim/ChangeLog b/sim/ChangeLog
index 7153c6a..1fe93df 100644
--- a/sim/ChangeLog
+++ b/sim/ChangeLog
@@ -1,5 +1,9 @@
 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
 
+	* mcore/interp.c (sim_do_command): Call freeargv() before return.
+
+2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
+
 	* common/sim-options.c (sim_args_command): Call freeargv() when
 	failure occurs.
 
diff --git a/sim/mcore/interp.c b/sim/mcore/interp.c
index d2edd12..dfaa6aa 100644
--- a/sim/mcore/interp.c
+++ b/sim/mcore/interp.c
@@ -2143,6 +2143,7 @@ sim_do_command (sd, cmd)
 	  if ((simargv[1] == NULL) || (simargv[2] == NULL))
 	    {
 	      fprintf (stderr, "Error: missing argument to watch cmd.\n");
+	      freeargv (simargv);
 	      return;
 	    }
 	  
@@ -2187,6 +2188,8 @@ sim_do_command (sd, cmd)
 	  fprintf (stderr,"Error: \"%s\" is not a valid M.CORE simulator command.\n",
 		   cmd);
 	}
+
+      freeargv (simargv);
     }
   else
     {
-- 
1.9.3

[PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

Or there will be memory leak.

2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>

	* microblaze/interp.c (sim_do_command): Call freeargv() before
	return.
---
 sim/ChangeLog           | 5 +++++
 sim/microblaze/interp.c | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/sim/ChangeLog b/sim/ChangeLog
index 1fe93df..582bd4a 100644
--- a/sim/ChangeLog
+++ b/sim/ChangeLog
@@ -1,5 +1,10 @@
 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
 
+	* microblaze/interp.c (sim_do_command): Call freeargv() before
+	return.
+
+2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
+
 	* mcore/interp.c (sim_do_command): Call freeargv() before return.
 
 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
diff --git a/sim/microblaze/interp.c b/sim/microblaze/interp.c
index 1c8a22d..4fc4595 100644
--- a/sim/microblaze/interp.c
+++ b/sim/microblaze/interp.c
@@ -1019,6 +1019,7 @@ sim_do_command (SIM_DESC sd, const char *cmd)
 	  if ((simargv[1] == NULL) || (simargv[2] == NULL))
 	    {
 	      fprintf (stderr, "Error: missing argument to watch cmd.\n");
+	      freeargv (simargv);
 	      return;
 	    }
 
@@ -1062,6 +1063,8 @@ sim_do_command (SIM_DESC sd, const char *cmd)
 	  fprintf (stderr,"Error: \"%s\" is not a valid M.CORE simulator command.\n",
 		   cmd);
 	}
+
+      freeargv (simargv);
     }
   else
     {
-- 
1.9.3

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Michael Eager]

On 01/30/15 15:07, Chen Gang S wrote:
> Or there will be memory leak.
>
> 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>
> 	* microblaze/interp.c (sim_do_command): Call freeargv() before
> 	return.

OK.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 1/31/15 09:14, Michael Eager wrote:
> On 01/30/15 15:07, Chen Gang S wrote:
>> Or there will be memory leak.
>>
>> 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>>

Oh, sorry, the ChangeLog should use 2015-01-31 instead of 2014-01-31 for
the all related 4 patches. I shall send patch v2 for them.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Joel Brobecker]

Chen,

> >> 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
> >>
> 
> Oh, sorry, the ChangeLog should use 2015-01-31 instead of 2014-01-31 for
> the all related 4 patches. I shall send patch v2 for them.

One important thing to watch out for: I think you may not have realized
that you pushed all 3 patches, whereas only the last one was approved.
So, the first two ones weren't expected to be pushed yet. If you need
help with git on how to push just the patches you want, let us know.
One way, for instance, is to work in a separate branch, and then
cherry-pick on master the patches only at the time where you want
to push them. That's what I do, and prevents this kind of accident
from happening.

In the meantime, since the patches have been pushed, I reviewed them
and they look good to me. So no need to revert. Normally, those are
reviewed by the sim maintainer, so he may have additional comments.

-- 
Joel

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 02/03/2015 10:50 AM, Joel Brobecker wrote:
> Chen,
> 
>>>> 2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>>>>
>>
>> Oh, sorry, the ChangeLog should use 2015-01-31 instead of 2014-01-31 for
>> the all related 4 patches. I shall send patch v2 for them.
> 
> One important thing to watch out for: I think you may not have realized
> that you pushed all 3 patches, whereas only the last one was approved.
> So, the first two ones weren't expected to be pushed yet. If you need
> help with git on how to push just the patches you want, let us know.
> One way, for instance, is to work in a separate branch, and then
> cherry-pick on master the patches only at the time where you want
> to push them. That's what I do, and prevents this kind of accident
> from happening.
> 

OK, thanks. I shall notice about it next time.

> In the meantime, since the patches have been pushed, I reviewed them
> and they look good to me. So no need to revert. Normally, those are
> reviewed by the sim maintainer, so he may have additional comments.
> 

OK, thanks.


-- 
Open, share, and attitude like air, water, and life which God blessed.

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On 31 Jan 2015 07:07, Chen Gang S wrote:
> --- a/sim/ChangeLog
> +++ b/sim/ChangeLog
> @@ -1,5 +1,10 @@
>  2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>  
> +	* microblaze/interp.c (sim_do_command): Call freeargv() before
> +	return.

this should be in sim/microblaze/ChangeLog instead.  it looks like your last 4 
entries in sim/ChangeLog need to get relocated.  please do so.

while you're there, you should also fix your gentmap.c entry in 
sim/common/ChangeLog -- only one space after the * is used.

as for the actual code, lgtm.  thanks for fixing up the various error paths.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 2/17/15 18:34, Mike Frysinger wrote:
> On 31 Jan 2015 07:07, Chen Gang S wrote:
>> --- a/sim/ChangeLog
>> +++ b/sim/ChangeLog
>> @@ -1,5 +1,10 @@
>>  2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>>  
>> +	* microblaze/interp.c (sim_do_command): Call freeargv() before
>> +	return.
> 
> this should be in sim/microblaze/ChangeLog instead.  it looks like your last 4 
> entries in sim/ChangeLog need to get relocated.  please do so.
> 

Oh, really, I shall change the related comments.

> while you're there, you should also fix your gentmap.c entry in 
> sim/common/ChangeLog -- only one space after the * is used.
> 

Oh, really, I shall change the related comments.

> as for the actual code, lgtm.  thanks for fixing up the various error paths.
> -mike

That what I should do, since I focus on binutils and gdb. :-)

And excuse me, I am not quite familiar with the related working flow.
Can I send 1 patch to fix the 2 comments, and "git push" it after it is
reviewed?.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

On 18 Feb 2015 07:18, Chen Gang S wrote:
> On 2/17/15 18:34, Mike Frysinger wrote:
> > On 31 Jan 2015 07:07, Chen Gang S wrote:
> >> --- a/sim/ChangeLog
> >> +++ b/sim/ChangeLog
> >> @@ -1,5 +1,10 @@
> >>  2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
> >>  
> >> +	* microblaze/interp.c (sim_do_command): Call freeargv() before
> >> +	return.
> > 
> > this should be in sim/microblaze/ChangeLog instead.  it looks like your last 4 
> > entries in sim/ChangeLog need to get relocated.  please do so.
> > 
> 
> Oh, really, I shall change the related comments.
> 
> > while you're there, you should also fix your gentmap.c entry in 
> > sim/common/ChangeLog -- only one space after the * is used.
> > 
> 
> Oh, really, I shall change the related comments.
> 
> > as for the actual code, lgtm.  thanks for fixing up the various error paths.
> 
> That what I should do, since I focus on binutils and gdb. :-)
> 
> And excuse me, I am not quite familiar with the related working flow.
> Can I send 1 patch to fix the 2 comments, and "git push" it after it is
> reviewed?.

generally the sim tree follows the gdb/binutils procedure.  i.e. people should 
post patches to the list and wait for approval from a relevant maintainer.  we 
don't generally get strict with the rules as long as you have good intentions 
and aren't breaking things (like compile failures) :).  so don't get too worried 
and feel free to ask as people don't mind helping guide newbies.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Chen Gang S]

On 2/18/15 08:22, Mike Frysinger wrote:
> On 18 Feb 2015 07:18, Chen Gang S wrote:
>> On 2/17/15 18:34, Mike Frysinger wrote:
>>> On 31 Jan 2015 07:07, Chen Gang S wrote:
>>>> --- a/sim/ChangeLog
>>>> +++ b/sim/ChangeLog
>>>> @@ -1,5 +1,10 @@
>>>>  2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>>>>  
>>>> +	* microblaze/interp.c (sim_do_command): Call freeargv() before
>>>> +	return.
>>>
>>> this should be in sim/microblaze/ChangeLog instead.  it looks like your last 4 
>>> entries in sim/ChangeLog need to get relocated.  please do so.
>>>
>>
>> Oh, really, I shall change the related comments.
>>
>>> while you're there, you should also fix your gentmap.c entry in 
>>> sim/common/ChangeLog -- only one space after the * is used.
>>>
>>
>> Oh, really, I shall change the related comments.
>>
>>> as for the actual code, lgtm.  thanks for fixing up the various error paths.
>>
>> That what I should do, since I focus on binutils and gdb. :-)
>>
>> And excuse me, I am not quite familiar with the related working flow.
>> Can I send 1 patch to fix the 2 comments, and "git push" it after it is
>> reviewed?.
> 
> generally the sim tree follows the gdb/binutils procedure.  i.e. people should 
> post patches to the list and wait for approval from a relevant maintainer.  we 
> don't generally get strict with the rules as long as you have good intentions 
> and aren't breaking things (like compile failures) :).  so don't get too worried 
> and feel free to ask as people don't mind helping guide newbies.
> -mike
> 

OK, thanks. I shall send one patch for the 2 comments, next.

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] sim: Be sure of calling freeargv() after successfully call buildargv().

[Michael Eager]

On 02/18/15 16:41, Chen Gang S wrote:
> On 2/18/15 08:22, Mike Frysinger wrote:
>> On 18 Feb 2015 07:18, Chen Gang S wrote:
>>> On 2/17/15 18:34, Mike Frysinger wrote:
>>>> On 31 Jan 2015 07:07, Chen Gang S wrote:
>>>>> --- a/sim/ChangeLog
>>>>> +++ b/sim/ChangeLog
>>>>> @@ -1,5 +1,10 @@
>>>>>   2014-01-31  Chen Gang <gang.chen.5i5j@gmail.com>
>>>>>
>>>>> +	* microblaze/interp.c (sim_do_command): Call freeargv() before
>>>>> +	return.
>>>>
>>>> this should be in sim/microblaze/ChangeLog instead.  it looks like your last 4
>>>> entries in sim/ChangeLog need to get relocated.  please do so.
>>>>
>>>
>>> Oh, really, I shall change the related comments.
>>>
>>>> while you're there, you should also fix your gentmap.c entry in
>>>> sim/common/ChangeLog -- only one space after the * is used.
>>>>
>>>
>>> Oh, really, I shall change the related comments.
>>>
>>>> as for the actual code, lgtm.  thanks for fixing up the various error paths.
>>>
>>> That what I should do, since I focus on binutils and gdb. :-)
>>>
>>> And excuse me, I am not quite familiar with the related working flow.
>>> Can I send 1 patch to fix the 2 comments, and "git push" it after it is
>>> reviewed?.
>>
>> generally the sim tree follows the gdb/binutils procedure.  i.e. people should
>> post patches to the list and wait for approval from a relevant maintainer.  we
>> don't generally get strict with the rules as long as you have good intentions
>> and aren't breaking things (like compile failures) :).  so don't get too worried
>> and feel free to ask as people don't mind helping guide newbies.
>> -mike
>>
>
> OK, thanks. I shall send one patch for the 2 comments, next.

If all you are changing is correcting the ChangeLog as indicated,
we can relax the normal procedure.  Please just check in fixes.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077