RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)

[Giuseppe MONTALTO <giuseppe dot montalto at st dot com>]

Thanks for your comments ,

the attached patch should fix all the issues.
Incidentally, I rebased as suggested, but found no issues with respect to:

    r = target_write_memory (addr, data, count);

regards,
	Giuseppe

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, September 26, 2012 10:44 PM
> To: Giuseppe MONTALTO
> Cc: Abid, Hafiz; gdb-patches@sourceware.org
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Giuseppe> Thanks for your comments: I've reworked the patch taking them
> Giuseppe> into account.  I have also updated it, so that it should now
> Giuseppe> fit the current HEAD
> 
> Thanks.
> 
> Giuseppe> +	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
> Giuseppe> +	parameter for pattern filling of memory regions
> 
> Capital "A" in "additional" here.
> 
> Your patch has a ^M character on each line...
> 
> Giuseppe> +  else
> Giuseppe> +    {
> Giuseppe> +      /* Pattern is equal to count:
> Giuseppe> +         we can just use databuf.  */
> Giuseppe> +      back_to = make_cleanup (xfree, data);
> Giuseppe> +      data = databuf;
> 
> This makes a cleanup referencing a garbage value of 'data' -- before 'data' is
> assigned.
> 
> Giuseppe> +  r = target_write_memory (addr, data, count);
> 
> There was a recent patch to change this line, so you may need to rebase
> again.
> 
> Giuseppe> +  xfree (databuf);
> 
> If you fix the above cleanup issue, then this will double-free databuf.
> 
> Giuseppe> -  if (parse->cmd->suppress_notification != NULL)
> Giuseppe> +  if (strncmp (parse->command, "break-", sizeof ("break-") -
> Giuseppe> + 1 ) == 0)
> Giuseppe>      {
> Giuseppe> -      make_cleanup_restore_integer (parse->cmd-
> >suppress_notification);
> Giuseppe> -      *parse->cmd->suppress_notification = 1;
> Giuseppe> +      make_cleanup_restore_integer
> (&mi_suppress_breakpoint_notifications);
> Giuseppe> +      mi_suppress_breakpoint_notifications = 1;
> Giuseppe>      }
> 
> This looks like an unintentional change.
> 
> Tom

Attachment: mi-fill-memory.patch
Description: mi-fill-memory.patch