Re: [PATCH 2/4] gdb.trace: Read XML target description from tfile.

[Pedro Alves <palves@redhat.com>]

On 02/06/2016 03:39 PM, Marcin Kościelnicki wrote:

>  
> +static void tfile_append_tdesc_line (const char *line);
>  static void tfile_interp_line (char *line,
>  			       struct uploaded_tp **utpp,
>  			       struct uploaded_tsv **utsvp);
> @@ -457,6 +462,12 @@ tfile_open (const char *arg, int from_tty)
>    trace_filename = xstrdup (filename);
>    trace_fd = scratch_chan;
>  
> +  /* Make sure this is clear.  */
> +  xfree (trace_tdesc);
> +  trace_tdesc = NULL;
> +  trace_tdesc_alloc = 0;
> +  trace_tdesc_len = 0;
> +
>    bytes = 0;
>    /* Read the file header and test for validity.  */
>    tfile_read ((gdb_byte *) &header, TRACE_HEADER_SIZE);
> @@ -505,6 +516,9 @@ tfile_open (const char *arg, int from_tty)
>  	    error (_("Excessively long lines in trace file"));
>  	}
>  
> +      /* We should have fetched tdesc by now.  */
> +      target_find_description ();

The comment makes it sounds like the call isn't needed.  ITYM
that the loop above should have read in the tdesc, if there was one,
right?  Could you reword the comment, please?

>  
>  static enum target_xfer_status
> +tfile_xfer_partial_features (struct target_ops *ops, const char *annex,
> +			     gdb_byte *readbuf, const gdb_byte *writebuf,
> +			     ULONGEST offset, ULONGEST len,
> +			     ULONGEST *xfered_len)
> +{
> +  if (strcmp (annex, "target.xml"))
> +    return TARGET_XFER_E_IO;
> +
> +  if (readbuf == NULL)
> +    error (_("tfile_xfer_partial: tdesc is read-only"));
> +
> +  if (!trace_tdesc)
> +    return TARGET_XFER_E_IO;

  if (trace_tdesc == NULL)
    return TARGET_XFER_E_IO;

>  
> +/* Handles tdesc lines from tfile by appending the payload to
> +   a global trace_tdesc variable.  */
> +
> +static void
> +tfile_append_tdesc_line (const char *line)
> +{
> +  int llen = strlen (line);
> +
> +  /* 2 chars for "\n\0".  */
> +  while (trace_tdesc_len + llen + 2 > trace_tdesc_alloc)
> +    {
> +      /* Grow the buffer.  */
> +      if (!trace_tdesc_alloc)
> +	trace_tdesc_alloc = 4096;
> +      else
> +	trace_tdesc_alloc *= 2;
> +      trace_tdesc = xrealloc(trace_tdesc, trace_tdesc_alloc);
> +    }
> +
> +  strcpy (trace_tdesc + trace_tdesc_len, line);
> +  trace_tdesc_len += llen;
> +  strcpy (trace_tdesc + trace_tdesc_len, "\n");
> +  trace_tdesc_len++;
> +}

Please use "struct buffer" instead of the manual xreallocing.

See similar examples here:
https://sourceware.org/ml/gdb-patches/2016-02/msg00070.html

Thanks,
Pedro Alves