[PATCH] gdbreplay: Calculate the checksum if missing

Tom Tromey tom@tromey.com
Tue Oct 22 18:02:08 GMT 2024


>>>>> "Alexandra" == Alexandra Hájková <ahajkova@redhat.com> writes:

Alexandra> +static void
Alexandra> +recalculate_csum (char *buf, int cnt, unsigned char *checksum_hex)

Normally functions have an intro comment.

BUF can be 'const char *'.

Alexandra> +{
Alexandra> +  int i;
Alexandra> +  unsigned char csum = 0;
Alexandra> +
Alexandra> +  for (i = 0; i < cnt;)
Alexandra> +    csum += buf[i++];

It's more normal to use 'for (int i = ...)' now, and also to put the
"++i" into the 'for', not the body.

Alexandra> +size_t line_len = 1024;
Alexandra> +char *line = (char *) malloc (line_len);

AFAICT 'line' is only used locally in 'play', so doesn't have to be a
global.

Also it would be preferable, IMO, to use some C++ type that handles
memory management, like std::string or std::vector.  This would help
with potential buffer overruns as well.

Alexandra> +  /* Packet starts with '+$' or '$', we don't want to calculate those
Alexandra> +     to the checksum, substract the offset to adjust the line length.  */
Alexandra> +  if (line[0] == '+')
Alexandra> +    offset = 2;

The comment mentions the '$' case but this isn't checked or handled
here.

Alexandra> +  if (where_csum == 0)
Alexandra> +    {
Alexandra> +      where_csum = len;
Alexandra> +      len += 3;
Alexandra> +      line[where_csum] = '#';

Like, can this overrun the 'line' buffer?

Tom


More information about the Gdb-patches mailing list