[RFC] Trailing spaces in solaris_pid_to_str

[RFC] Trailing spaces in solaris_pid_to_str

[Doug Evans]

I can imagine that the trailing spaces are so the output of "info threads"
looks pretty, but these trailing spaces would mess up the text when used
elsewhere.

[Assuming the trailing spaces are indeed for prettier "info threads" ...]
If one wants the output of "info threads" to be prettier in this way
then it seems like the right thing to do is have the "info threads"
printer add the spaces.  (a) info threads presumably knows more about
how much alignment is needed, and (b) all ports would win.

If the spaces are to line these all up:
"Thread %ld (defunct)"
"Thread %ld (LWP %ld)"
"Thread %ld        "
"LWP    %ld        "
"process %d    "
they're already in need of some TLC.

Doesn't matter to me whether to check this in or not,
just thought I'd pass it on.  It is odd.  Is there another reason
for these spaces that I'm missing?

[The spaces in "LWP    %ld" are to line up the text with "Thread %ld".
They don't bother me as much as the trailing spaces so I left them in,
but they could just as well be deleted.]

2009-05-01  Doug Evans  <dje@google.com>

	* sol-thread.c (solaris_pid_to_str): Remove trailing spaces in result.

Index: sol-thread.c
===================================================================
RCS file: /cvs/src/src/gdb/sol-thread.c,v
retrieving revision 1.72
diff -u -p -r1.72 sol-thread.c
--- sol-thread.c	23 Feb 2009 00:03:50 -0000	1.72
+++ sol-thread.c	1 May 2009 21:19:50 -0000
@@ -1152,12 +1152,12 @@ solaris_pid_to_str (struct target_ops *o
 	sprintf (buf, "Thread %ld (LWP %ld)",
 		 GET_THREAD (ptid), GET_LWP (lwp));
       else
-	sprintf (buf, "Thread %ld        ", GET_THREAD (ptid));
+	sprintf (buf, "Thread %ld", GET_THREAD (ptid));
     }
   else if (GET_LWP (ptid) != 0)
-    sprintf (buf, "LWP    %ld        ", GET_LWP (ptid));
+    sprintf (buf, "LWP    %ld", GET_LWP (ptid));
   else
-    sprintf (buf, "process %d    ", PIDGET (ptid));
+    sprintf (buf, "process %d", PIDGET (ptid));
 
   return buf;
 }

Re: [RFC] Trailing spaces in solaris_pid_to_str

[Joel Brobecker]

> Doesn't matter to me whether to check this in or not,
> just thought I'd pass it on.  It is odd.  Is there another reason
> for these spaces that I'm missing?

Can't figure it out. I did a bit of archeology, and this pre-dates
the public CVS.

> 2009-05-01  Doug Evans  <dje@google.com>
> 
> 	* sol-thread.c (solaris_pid_to_str): Remove trailing spaces in result.

I'm not objecting, but if it's just a visual annoyance and no one else
provides feedback, then perhaps it's better to leave things as is
(taste varies from person to person). If you need that function
elsewhere and it's causing trouble, on the other hand...

> [The spaces in "LWP    %ld" are to line up the text with "Thread %ld".
> They don't bother me as much as the trailing spaces so I left them in,
> but they could just as well be deleted.]

If you do remove the trailing spaces, might as well remove the leading
ones too. There're there for formatting, so if we remove the formatting,
we might as well remove all of it (IMO).

-- 
Joel

Re: [RFC] Trailing spaces in solaris_pid_to_str

[Pedro Alves]

On Tuesday 12 May 2009 10:19:45, Joel Brobecker wrote:
> > Doesn't matter to me whether to check this in or not,
> > just thought I'd pass it on.  It is odd.  Is there another reason
> > for these spaces that I'm missing?
> 
> Can't figure it out. I did a bit of archeology, and this pre-dates
> the public CVS.

Yes, if you do "info threads", it becomes dead obvious it's just for
lining up.  From PR9242, here's what one gets on solaris:

  6 Thread 3          0xdfa39657 in _swtch () from /usr/lib/libthread.so.1
  5 Thread 2 (LWP 2)  0xdfadb5a8 in _signotifywait () from /usr/lib/libc.so.1
* 4 Thread 1 (LWP 1)  main (argc=0x1, argv=0x8047874)
    at /users/pes/gdbnd/devo/gdb/testsuite/gdb.threads/pthreads.c:122
  3 LWP    3          0xdfa3a158 in _lwp_start () from /usr/lib/libthread.so.1
  2 LWP    2          0xdfadb5a8 in _signotifywait () from /usr/lib/libc.so.1
  1 LWP    1          main (argc=0x1, argv=0x8047874)
    at /users/pes/gdbnd/devo/gdb/testsuite/gdb.threads/pthreads.c:122

http://sourceware.org/bugzilla/show_bug.cgi?id=9242

Hmmm, BTW, I've fixed that PR recently; time to close it.

I've no real opinion on if the lining up is desirable or not.

> > 2009-05-01  Doug Evans  <dje@google.com>
> > 
> > 	* sol-thread.c (solaris_pid_to_str): Remove trailing spaces in result.
> 
> I'm not objecting, but if it's just a visual annoyance and no one else
> provides feedback, then perhaps it's better to leave things as is
> (taste varies from person to person). If you need that function
> elsewhere and it's causing trouble, on the other hand...
> 
> > [The spaces in "LWP    %ld" are to line up the text with "Thread %ld".
> > They don't bother me as much as the trailing spaces so I left them in,
> > but they could just as well be deleted.]
> 
> If you do remove the trailing spaces, might as well remove the leading
> ones too. There're there for formatting, so if we remove the formatting,
> we might as well remove all of it (IMO).


-- 
Pedro Alves