This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Tracepoint source strings


Eli Zaretskii wrote:
struct breakpoint *
create_tracepoint_from_upload (struct uploaded_tp *utp)
{
! char *addr_str, small_buf[100];
[...]
! sprintf (small_buf, "*%s", hex_string (utp->addr));

Tz-tz-tz... Using a constant-size buffer in sprintf without any check
for overflow? Are you sure that calling the buffer ``small'' will
magically keep you from trouble? ;-)

Presumably even a hypothetical future 128-bit address won't need more than 65 chars to print. :-)


! if (utp->cond && !utp->cond_string)
! warning ("Uploaded tracepoint %d condition has no source form, ignoring it",

What about _() ?



Oops, yeah.



+ void
+ encode_source_string (int tpnum, ULONGEST addr,
+ char *srctype, char *src, char *buf)
+ {
+ sprintf (buf, "%x:%s:%s:%x:%x:",
+ tpnum, phex_nz (addr, sizeof (addr)), srctype, 0, (int) strlen (src));

Again, sprintf on a buffer whose size is not even known.

Hmmm, I'll see what I can do.


written = fwrite ("\x7fTRACE0\n", 8, 1, fp);
! if (written < 8)
perror_with_name (pathname);
/* Write descriptive info. */
--- 2468,2474 ----
binary file, plus a hint as what this file is, and a version
number in case of future needs. */
written = fwrite ("\x7fTRACE0\n", 8, 1, fp);
! if (written < 1)
perror_with_name (pathname);

Why did you change this to accept partial writes?

I was hoping to fix a major brain cramp of mine without anybody noticing - oh well. :-) The two numeric arguments to fwrite are semi-redundant a la calloc, and the return result is based on the *second* argument, which is the number of "items". The other way to fix is to swap the two arguments; about the only advantage of this way is that we always test against 1, instead of repeating the fwrite argument.


+ Specify a source string of tracepoint @var{n} at address @var{var}.
                                                             ^^^^^^^^^
You meant @var{addr}, I presume.

Anyway, can we have several tracepoints with the same number? If not,
why do we need to give the address as well?

Yes and yes.


such as @samp{cond} for the
+ conditional expression

Conditional expression for what or from where? I'm guessing this is
somehow related to the original definition of the tracepoint, but
please connect the dots more explicitly.

Yes, it's the tracepoint's condition, I can make an xref. I tend to think we can be a little more abbreviated when describing the protocol, because presumably anybody writing a target agent is going to know GDB concepts pretty well.


+ @value{GDBN} issues a separate packet, in order, for each command in a
+ list.

What list?

Command list.


The target does not need to do anything with source strings
+ except report them back as part of the @samp{qTfP}/@samp{qTsP}
+ queries.

"As part of queries" or as part or _responses_?

As part of replies. I've glossed over details of the qT[fs]P reply to date, so as to not to have to rewrite so much as the rest of the tracepoint patches go in. The grand plan is that there is a notion of "tracepoint description" with a syntax that is common between remote download, remote upload, and trace file, and it should go into its own section that is referenced from both protocol and file format descriptions.


Stan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]