On 1/30/19 10:48 PM, Lucien Murray-Pitts wrote: > The result is that vCont now does not recognise the case where no > process/thread is provided after the action.
Thanks for re-sending; this one came through a lot nicer! Welcome to the community; while a first submission can always be a bit daunting, the maintainers generally will help fix things up as you refine your process for making future submissions run more smoothly. My next suggestion: wrap your commit message body around 65-70 columns, rather than using long lines. While it may sound archaic, many developers still use 80-column terminals, and 'git log' adds indentation; long lines become difficult to read if you don't manually wrap them. > > This may not show up with GDB, but using Lauterbach Trace32, and Hexrays IDA > Pro this issue is immediately seen. > The response is a "$#00" empty packet, showing it is unsupported packet. > > This is defined in the RSP document as "An action with no thread-id matches > all threads." > (https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#vCont-packet ) > > Thus the valid vCont packets now are as below, however parsing is still not > very strict. > vCont;c/s - Step/Continue all threads > vCont;c/s:[pX.]Y - Step/Continue optional process X, thread Y > vCont;C##/S##:[pX.]Y - Step/Continue with signal ## on optional > process X, thread Y > * If X or Y are -1 then it applies the action to all processes/threads. > > Signed-off-by: Lucien Murray-Pitts <lucienmp_antis...@yahoo.com> > --- > gdbstub.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) I'll let someone more familiar with gdbstub review the actual patch content for correctness, but I'll do a quick pass for style: > > diff --git a/gdbstub.c b/gdbstub.c > index bfc7afb509..ce0dde2e24 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1169,6 +1169,7 @@ static int is_query_packet(const char *p, const char > *query, char separator) > */ > static int gdb_handle_vcont(GDBState *s, const char *p) > { > + GDBThreadIdKind vcontThreadType ; No space before ; > int res, signal = 0; > char cur_action; > char *newstates; > @@ -1218,12 +1219,23 @@ static int gdb_handle_vcont(GDBState *s, const char > *p) > goto out; > } > > - if (*p++ != ':') { > + /* > + * In the case we have vCont;c or vCont;s - action is on all threads Grammar suggestion: s/In the case/When/, and use a trailing '.' to end the sentence. > + * Alternatively vCont;c;s:p1.1 is a possible, but meaningless > format, > + * And in the else the "vCont;c:p1.1;... format is supported. s/And in the else/otherwise/ > + */ > + if (*p == '\0' || *p == ';') { > + vcontThreadType = GDB_ALL_THREADS ; > + pid = 1 ; > + tid = 1 ; Again, no space before ; > + } else if (*p++ == ':') { > + vcontThreadType = read_thread_id(p, &p, &pid, &tid) ; > + } else { > res = -ENOTSUP; > goto out; > } > > - switch (read_thread_id(p, &p, &pid, &tid)) { > + switch (vcontThreadType) { > case GDB_READ_THREAD_ERR: > res = -EINVAL; > goto out; > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
