Re: [PATCH] new MI command for pattern filling of memory regions

Tom Tromey
2012-05-11T14:36:15Z

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> + if (argc == 3)
Giuseppe> + count = strtol(argv[2], NULL, 10);
Giuseppe> + else
Giuseppe> + count = (long int)len;

The indentation is wrong here and elsewhere in the patch.
There are missing spaces as well.
See the GNU Coding Standards for details.

I don't think the cast here, or other casts in the patch, are necessary.

Giuseppe> + /* pattern is made of less bytes than count:
Giuseppe> + repeat pattern to fill memory. */
Giuseppe> + data = xmalloc (count/len);

This allocates count/len bytes...

Giuseppe> + steps = count/len;
Giuseppe> + for (j = 0; j < steps; j++)
Giuseppe> + {
Giuseppe> + for (i = 0; i < len; ++i)
Giuseppe> + {
Giuseppe> + int x;
Giuseppe> + sscanf (cdata + i * 2, "%02x", &x);
Giuseppe> + data[i + j * len] = (gdb_byte)x;

... but writes 'count' bytes.

Re-invoking sscanf on each iteration seems odd to me. I think it would
be cheaper to do this loop once and then memcpy the bits into place.

Tom