From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37663 invoked by alias); 26 Jul 2017 10:49:28 -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 37439 invoked by uid 89); 26 Jul 2017 10:49:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lf0-f54.google.com Received: from mail-lf0-f54.google.com (HELO mail-lf0-f54.google.com) (209.85.215.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Jul 2017 10:49:25 +0000 Received: by mail-lf0-f54.google.com with SMTP id g25so62935769lfh.1 for ; Wed, 26 Jul 2017 03:49:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=0DDjsA/zy75Nq5cM1DANmDoE/i4IbXGnm5fjGC8qeTg=; b=SyqBWBUbS6CC0IwgNovTNxfE+LYTgcuuf2aopvIJuJk8qmqX+Rqpt4wBZr7cYYZwfv PISCNMdE/7s20aHNYldf5o1KzDKQniGo5VLH7GVKkftyon4/onSTssbuNNGojTO2pAbE 4/YeWapgQGTXZKg2+asm0eNaNfRzhPQCNNC3KNAqUU4QrOk9tiSal49a73xuUqFqnbcX knoxFjipopL2aoiVNLBtRaWDSBPeWtaUNLcIF42Pqu2w5Qp5xFePxarTts1BW12pNXhv WTLaWN/24Y2IMYExmdVAGPrLb97THd9rftRlMofHpOnbWZXy8jyO/lduEw9AQzZjegK3 DFEg== X-Gm-Message-State: AIVw111vf87qhIrdtyYbJX9IIRp64671g4SomUu8PK7dT9dGGfh9hyVM 8gGIOnlEmPEZ0Vcw X-Received: by 10.25.56.27 with SMTP id f27mr240589lfa.167.1501066162034; Wed, 26 Jul 2017 03:49:22 -0700 (PDT) Received: from [192.168.18.151] ([81.222.86.72]) by smtp.gmail.com with ESMTPSA id h15sm2880298lfk.31.2017.07.26.03.49.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 26 Jul 2017 03:49:21 -0700 (PDT) Subject: Re: [PATCH v2] Align natural-format register values to the same column To: Simon Marchi Cc: gdb-patches@sourceware.org References: <1501055625-25029-1-git-send-email-b7.10110111@gmail.com> <5bce7397bf829abb601f61f6e40877c8@polymtl.ca> From: Ruslan Kabatsayev Message-ID: <234e1432-11fe-0362-41cf-1096dfb38b9b@gmail.com> Date: Wed, 26 Jul 2017 10:49:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <5bce7397bf829abb601f61f6e40877c8@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-07/txt/msg00409.txt.bz2 Hi Simon, On 26.07.2017 12:33, Simon Marchi wrote: > Hi Ruslan, > > Thanks for the update, I just have some minor comments, but otherwise > the patch looks good to me. > > On 2017-07-26 09:53, Ruslan Kabatsayev wrote: >> Currently, commands such as "info reg", "info all-reg", as well as >> register >> window in the TUI print badly aligned columns, like here: >> >> eax 0x1 1 >> ecx 0xffffd3e0 -11296 >> edx 0xffffd404 -11260 >> ebx 0xf7fa5ff4 -134586380 >> esp 0xffffd390 0xffffd390 >> ebp 0xffffd3c8 0xffffd3c8 >> esi 0x0 0 >> edi 0x0 0 >> eip 0x8048b60 0x8048b60 >> eflags 0x286 [ PF SF IF ] >> cs 0x23 35 >> ss 0x2b 43 >> ds 0x2b 43 >> es 0x2b 43 >> fs 0x0 0 >> gs 0x63 99 >> >> After this patch, these commands print the third column values >> consistently >> aligned one under another, provided the second column is not too long. >> Originally, the third column was (attempted to be) aligned using a >> simple tab >> character. Lots of tests in the test suite check for it, so this patch >> retains >> the tab in the output after the second column. This allows these tests >> to >> continue working unchanged. What is different is that now the tab may >> be >> followed by several spaces, which complete the task of aligning the >> third >> column when the sole tab doesn't work well. >> >> gdb/ChangeLog: >> >> * infcmd.c (default_print_one_register_info): Align natural- >> format column value consistently one after another. >> --- >> gdb/infcmd.c | 35 ++++++++++++++++++++++++++--------- >> 1 file changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index defa7b0..56cdee2 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -2270,6 +2270,15 @@ path_command (char *dirname, int from_tty) >> } >> >> > > Please add a short comment to explain what this function does. > >> +static void >> +pad_to_column_prepending_tab (string_file& stream, int col) > > The & should go after the space. > >> +{ >> + const int size_with_tab = stream.size () / 8 * 8 + 8; >> + stream.putc ('\t'); >> + if (size_with_tab < col) >> + stream.puts (n_spaces (col - size_with_tab)); >> +} >> + >> /* Print out the register NAME with value VAL, to FILE, in the default >> fashion. */ >> >> @@ -2280,9 +2289,15 @@ default_print_one_register_info (struct ui_file >> *file, >> { >> struct type *regtype = value_type (val); >> int print_raw_format; >> + string_file format_stream; >> + enum tab_stops >> + { >> + value_column_1=15, >> + value_column_2=32, > > Perhaps those could have more explicit names, like "column_hex" and > "column_natural"? This was my first thought, but then I noticed that for floating-point registers we have "(raw 0x....)" on the right column and "1.32536e+52" on the left one. Do you still think it's OK to name the left column hex and the right one natural? > > Formatting nit: indent those two lines with two columns less (so with 6 > spaces in total) and add spaces around the =. > >> + }; >> >> - fputs_filtered (name, file); >> - print_spaces_filtered (15 - strlen (name), file); >> + format_stream.puts (name); >> + format_stream.puts (n_spaces (value_column_1 - strlen (name))); >> >> print_raw_format = (value_entirely_available (val) >> && !value_optimized_out (val)); >> @@ -2301,14 +2316,15 @@ default_print_one_register_info (struct ui_file >> *file, >> >> val_print (regtype, >> value_embedded_offset (val), 0, >> - file, 0, val, &opts, current_language); >> + &format_stream, 0, val, &opts, current_language); >> >> if (print_raw_format) >> { >> - fprintf_filtered (file, "\t(raw "); >> - print_hex_chars (file, valaddr, TYPE_LENGTH (regtype), byte_order, >> + pad_to_column_prepending_tab (format_stream, value_column_2); >> + format_stream.puts ("(raw "); >> + print_hex_chars (&format_stream, valaddr, TYPE_LENGTH (regtype), >> byte_order, >> true); >> - fprintf_filtered (file, ")"); >> + format_stream.puts (")"); >> } >> } >> else >> @@ -2320,20 +2336,21 @@ default_print_one_register_info (struct ui_file >> *file, >> opts.deref_ref = 1; >> val_print (regtype, >> value_embedded_offset (val), 0, >> - file, 0, val, &opts, current_language); >> + &format_stream, 0, val, &opts, current_language); >> /* If not a vector register, print it also according to its >> natural format. */ >> if (print_raw_format && TYPE_VECTOR (regtype) == 0) >> { >> + pad_to_column_prepending_tab (format_stream, value_column_2); >> get_user_print_options (&opts); >> opts.deref_ref = 1; >> - fprintf_filtered (file, "\t"); >> val_print (regtype, >> value_embedded_offset (val), 0, >> - file, 0, val, &opts, current_language); >> + &format_stream, 0, val, &opts, current_language); >> } >> } >> >> + fputs_filtered (format_stream.c_str (), file); >> fprintf_filtered (file, "\n"); >> } > > Thanks, > > Simon > Regards, Ruslan