This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PING][RFC-v4] Add windows OS Thread Information Block
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 01 Apr 2010 16:30:13 +0300
- Subject: Re: [PING][RFC-v4] Add windows OS Thread Information Block
- References: <000901c9f5ef$4ee06f10$eca14d30$@u-strasbg.fr> <201003101725.48298.pedro@codesourcery.com> <000c01cac0a0$3935fbe0$aba1f3a0$%muller@ics-cnrs.unistra.fr> <201003110000.31184.pedro@codesourcery.com> <002101cac0f2$a2298890$e67c99b0$%muller@ics-cnrs.unistra.fr> <000e01cac488$27dcf970$7796ec50$%muller@ics-cnrs.unistra.fr> <001201cad17f$6a058980$3e109c80$%muller@ics-cnrs.unistra.fr>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Date: Thu, 1 Apr 2010 11:40:50 +0200
>
> I am still waiting for a comment from any global
> maintainer concerning the non-(windows specific) parts
> of that patch. Christopher approved the windows part.
Sorry I missed that.
> > + if (target_get_tib_address (ptid, &thread_local_base) == 0)
> > + {
> > + printf_filtered ("Unable to get thread local base for ThreadId
> > %s\n",
> > + pulongest (ptid_get_tid(ptid)));
Please add _() around the user messages.
> > + printf_filtered ("Unable to read thread information block for
> > ThreadId %s at address %s\n",
> > + pulongest (ptid_get_tid (ptid)),
> > + paddress (target_gdbarch, thread_local_base));
Ditto.
> > + printf_filtered ("Thread Information Block %s at %s\n",
> > + pulongest (ptid_get_tid (ptid)),
> > + paddress (target_gdbarch, thread_local_base));
Ditto.
> > + printf_filtered ("%s is 0x%s\n", TIB_NAME [i], phex (val, size));
> > + else if (val != 0)
> > + printf_filtered ("TIB[0x%s] is 0x%s\n", phex (i*size, 2),
> > + phex (val, size));
Ditto.
> > + add_prefix_cmd ("w32", class_info, info_w32_command,
> > + _("Print information specific to Win32 debugging."),
RMS would say "don't call Windows ``a win''".
> > +If enabled, all non-zero fields of thread information block are displayed,\n\
> > +even if its meaning is unknown."),
"its" is inappropriate here, as "fields" are in plural. I suggest
"their" instead.
> > +@item $_tlb
> > +@vindex $_tlb@r{, convenience variable}
> > +The variable @code{$_tlb} is automatically set for Windows OS running
> > +applications in native mode or connected to a gdbserver that supports
This is backwards: it makes it sound like we set the variable for
Windows, not for the application. I suggest the following alternative
wording:
The variable @code{$_tlb} is automatically set when debugging
applications running on MS-Windows in native mode or connected to
gdbserver that supports the @code{qGetTIBAddr} request.
Please also add an @xref to where the qGetTIBAddr packet is
described.
> > +@code{qGetTIBAddr} requests. This variable contains the address of the
^^
Two spaces between sentences, please.
> > +@tab Display Windows OS Thread Information Block.
"Display MS-Windows Thread Information Block."
> > +An error occured. This means that either the thread was not found, or
^^
Two spaces.
The patch for the manual is okay with the above changes.