From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6621 invoked by alias); 18 Sep 2013 18:37:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 6609 invoked by uid 89); 18 Sep 2013 18:37:19 -0000 Received: from mail-vb0-f54.google.com (HELO mail-vb0-f54.google.com) (209.85.212.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 18 Sep 2013 18:37:19 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,KHOP_THREADED,NO_RELAYS autolearn=ham version=3.3.2 X-HELO: mail-vb0-f54.google.com Received: by mail-vb0-f54.google.com with SMTP id q14so5792481vbe.27 for ; Wed, 18 Sep 2013 11:37:16 -0700 (PDT) X-Received: by 10.220.169.203 with SMTP id a11mr1821893vcz.26.1379529436255; Wed, 18 Sep 2013 11:37:16 -0700 (PDT) MIME-Version: 1.0 Reply-To: pat.pannuto@gmail.com Received: by 10.58.144.101 with HTTP; Wed, 18 Sep 2013 11:36:55 -0700 (PDT) In-Reply-To: References: From: Pat Pannuto Date: Wed, 18 Sep 2013 18:37:00 -0000 Message-ID: Subject: Re: [PATCH] Print thread name when executing thread commands To: Ricard Wanderlof Cc: "gdb-patches@sourceware.org" Content-Type: multipart/mixed; boundary=047d7b6721c4015b9304e6acba5e X-SW-Source: 2013-09/txt/msg00618.txt.bz2 --047d7b6721c4015b9304e6acba5e Content-Type: text/plain; charset=ISO-8859-1 Content-length: 3921 On Wed, Sep 18, 2013 at 3:04 AM, Ricard Wanderlof wrote: > > On Wed, 18 Sep 2013, Pat Pannuto wrote: > >> Currently the thread family of commands only prints the thread ID and >> PID. GDB also has access to the thread's name, which is often an easier >> way of quickly identifying a thread. This simple patch uses the same >> mechanism as `info threads' to get the name of a thread and add it to >> the thread identifier line when it's printed. >> >> I wrapped the name in []'s to offset it and make it visually distinct >> from the ()'s that wrap the other thread information. I'm not at all >> attached to this look, just looked best to me. >> >> Diff should be from current cvs. This is my first patch to GDB, if I >> messed something up don't hesitate to yell at me. > > > I'm fairly new here too so maybe I shouldn't say too much, but supplying the > patch inline (including the ChangeLog entry) in the post makes it much > easier to comment on it. I still like adding it as an attachement lest the > mail program messes up the formatting so it doesn't apply. > Done. > My initial comment is that the ChangeLog format requires that it should be > more specific regarding which functions have been modified. One should be > able to grep for the function name in the ChangeLog. Something like > > 2013-09-18 Pat Pannuto > > * thread.c (thread_apply_all_command, thread_apply_command): > Print thread name as well as ID number and PID. > Makes sense. Updated. > In the code, there was a 'char *name' in one place, but 'char* name' in > another. Looking quickly, the indentation of multiple-line statements looked > inconsistent in some places too. I fixed the misplaced *. As for the spacing, it follows the spacing of the lines around them AFAICT. > > As for the functionality itself, I'm not in a position to approve but I > think it's a good idea. It's annoying just to get the thread id when there's > more information available. I don't know if there are any caveats. > > /Ricard Updated ChangeLog, diff: $ cat print_thread_names.ChangeLog 2013-09-18 Pat Pannuto * thread.c (thread_apply_all_command, thread_apply_command): Print thread name as well as ID number and PID. $ cat print_thread_names.diff Index: gdb/thread.c =================================================================== RCS file: /cvs/src/src/gdb/thread.c,v retrieving revision 1.157 diff -u -p -r1.157 thread.c --- gdb/thread.c 17 Sep 2013 18:26:39 -0000 1.157 +++ gdb/thread.c 18 Sep 2013 18:31:15 -0000 @@ -1255,10 +1255,14 @@ thread_apply_all_command (char *cmd, int for (k = 0; k != i; k++) if (thread_alive (tp_array[k])) { + char *name; switch_to_thread (tp_array[k]->ptid); - printf_filtered (_("\nThread %d (%s):\n"), + name = tp_array[k]->name ? tp_array[k]->name + : target_thread_name (tp_array[k]); + printf_filtered (_("\nThread %d (%s) [%s]:\n"), tp_array[k]->num, - target_pid_to_str (inferior_ptid)); + target_pid_to_str (inferior_ptid), + name); execute_command (cmd, from_tty); /* Restore exact command used previously. */ @@ -1308,10 +1312,12 @@ thread_apply_command (char *tidlist, int warning (_("Thread %d has terminated."), start); else { + char *name; switch_to_thread (tp->ptid); - printf_filtered (_("\nThread %d (%s):\n"), tp->num, - target_pid_to_str (inferior_ptid)); + name = tp->name ? tp->name : target_thread_name (tp); + printf_filtered (_("\nThread %d (%s) [%s]:\n"), tp->num, + target_pid_to_str (inferior_ptid), name); execute_command (cmd, from_tty); /* Restore exact command used previously. */ --047d7b6721c4015b9304e6acba5e Content-Type: application/octet-stream; name="print_thread_names.ChangeLog" Content-Disposition: attachment; filename="print_thread_names.ChangeLog" Content-Transfer-Encoding: base64 X-Attachment-Id: f_hlqw6bic0 Content-length: 220 MjAxMy0wOS0xOCAgUGF0IFBhbm51dG8gPHBhdC5wYW5udXRvQGdtYWlsLmNv bT4KCgkqIHRocmVhZC5jICh0aHJlYWRfYXBwbHlfYWxsX2NvbW1hbmQsIHRo cmVhZF9hcHBseV9jb21tYW5kKToKCVByaW50IHRocmVhZCBuYW1lIGFzIHdl bGwgYXMgSUQgbnVtYmVyIGFuZCBQSUQuCgo= --047d7b6721c4015b9304e6acba5e Content-Type: application/octet-stream; name="print_thread_names.diff" Content-Disposition: attachment; filename="print_thread_names.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_hlqw6h0v1 Content-length: 2054 SW5kZXg6IGdkYi90aHJlYWQuYwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09ClJD UyBmaWxlOiAvY3ZzL3NyYy9zcmMvZ2RiL3RocmVhZC5jLHYKcmV0cmlldmlu ZyByZXZpc2lvbiAxLjE1NwpkaWZmIC11IC1wIC1yMS4xNTcgdGhyZWFkLmMK LS0tIGdkYi90aHJlYWQuYwkxNyBTZXAgMjAxMyAxODoyNjozOSAtMDAwMAkx LjE1NworKysgZ2RiL3RocmVhZC5jCTE4IFNlcCAyMDEzIDE4OjMxOjE1IC0w MDAwCkBAIC0xMjU1LDEwICsxMjU1LDE0IEBAIHRocmVhZF9hcHBseV9hbGxf Y29tbWFuZCAoY2hhciAqY21kLCBpbnQKICAgICAgIGZvciAoayA9IDA7IGsg IT0gaTsgaysrKQogICAgICAgICBpZiAodGhyZWFkX2FsaXZlICh0cF9hcnJh eVtrXSkpCiAgICAgICAgICAgeworICAgICAgICAgICAgY2hhciAqbmFtZTsK ICAgICAgICAgICAgIHN3aXRjaF90b190aHJlYWQgKHRwX2FycmF5W2tdLT5w dGlkKTsKLSAgICAgICAgICAgIHByaW50Zl9maWx0ZXJlZCAoXygiXG5UaHJl YWQgJWQgKCVzKTpcbiIpLCAKKyAgICAgICAgICAgIG5hbWUgPSB0cF9hcnJh eVtrXS0+bmFtZSA/IHRwX2FycmF5W2tdLT5uYW1lCisgICAgICAgICAgICAg ICAgICAgIDogdGFyZ2V0X3RocmVhZF9uYW1lICh0cF9hcnJheVtrXSk7Cisg ICAgICAgICAgICBwcmludGZfZmlsdGVyZWQgKF8oIlxuVGhyZWFkICVkICgl cykgWyVzXTpcbiIpLCAKIAkJCSAgICAgdHBfYXJyYXlba10tPm51bSwKLQkJ CSAgICAgdGFyZ2V0X3BpZF90b19zdHIgKGluZmVyaW9yX3B0aWQpKTsKKwkJ CSAgICAgdGFyZ2V0X3BpZF90b19zdHIgKGluZmVyaW9yX3B0aWQpLAorCQkJ ICAgICBuYW1lKTsKICAgICAgICAgICAgIGV4ZWN1dGVfY29tbWFuZCAoY21k LCBmcm9tX3R0eSk7CiAKICAgICAgICAgICAgIC8qIFJlc3RvcmUgZXhhY3Qg Y29tbWFuZCB1c2VkIHByZXZpb3VzbHkuICAqLwpAQCAtMTMwOCwxMCArMTMx MiwxMiBAQCB0aHJlYWRfYXBwbHlfY29tbWFuZCAoY2hhciAqdGlkbGlzdCwg aW50CiAJd2FybmluZyAoXygiVGhyZWFkICVkIGhhcyB0ZXJtaW5hdGVkLiIp LCBzdGFydCk7CiAgICAgICBlbHNlCiAJeworCSAgY2hhciAqbmFtZTsKIAkg IHN3aXRjaF90b190aHJlYWQgKHRwLT5wdGlkKTsKIAotCSAgcHJpbnRmX2Zp bHRlcmVkIChfKCJcblRocmVhZCAlZCAoJXMpOlxuIiksIHRwLT5udW0sCi0J CQkgICB0YXJnZXRfcGlkX3RvX3N0ciAoaW5mZXJpb3JfcHRpZCkpOworCSAg bmFtZSA9IHRwLT5uYW1lID8gdHAtPm5hbWUgOiB0YXJnZXRfdGhyZWFkX25h bWUgKHRwKTsKKwkgIHByaW50Zl9maWx0ZXJlZCAoXygiXG5UaHJlYWQgJWQg KCVzKSBbJXNdOlxuIiksIHRwLT5udW0sCisJCQkgICB0YXJnZXRfcGlkX3Rv X3N0ciAoaW5mZXJpb3JfcHRpZCksIG5hbWUpOwogCSAgZXhlY3V0ZV9jb21t YW5kIChjbWQsIGZyb21fdHR5KTsKIAogCSAgLyogUmVzdG9yZSBleGFjdCBj b21tYW5kIHVzZWQgcHJldmlvdXNseS4gICovCg== --047d7b6721c4015b9304e6acba5e--