Re: [RFA-v8] Add windows OS Thread Information Block

[Pedro Alves <pedro at codesourcery dot com>]

On Tuesday 13 April 2010 14:21:09, Pierre Muller wrote:

> > +  Return the address of the Windows Thread Information Block of the
> > +  current thread.

This one's my fault: s/of the current/of a given/ please.  The thread
is explicitly specified in the packet.  No need to repost for this.

>   This is perfect for me, inserted as is.

> > If you would please install a real `show' callback instead of this
> > second NULL, I'd appreciate it.  The reason is that the default
> > callback
> > when this is left as NULL can't work correctly in non-English.  A goal
> > is to have no command left with this callback left as NULL.
> > But if you don't want to do it now (too many iterations already!), it's
> > quite fine.
> 
>   I wrote something, but I didn't really understand why 
> the string that is the fifth argument doesn't allow proper
> internalization...

Thanks.  The default callback (deprecated_show_value_hack) literally
skips 5 characters, strips the whitespace, and then uppercases the
following letter.  Then it appends "is <value>".  So, in your example:

  "Show whether to display all non-zero fields of thread information block"

is auto-transformed to:

  "Whether to display all non-zero fields of thread information block is <value>"

If the string is translated to non-English, this isn't going to work correctly.

> gdbserver/ChangeLog entry:
> 
> 	* server.c (handle_query): Acknowledge support
> 	for 'qGetTIBAddr' if get_tib_addr field of the_target
> 	is set.

This first sentence is no longer true.  May be good to go through
the whole changelog before committing.  Some more bits may be
out of date; I'm not going to check that myself.


One final thing you need to address when committing.  In gdbserver:

> +  /* Read Thread Information Block address.  */
> +  int (*get_tib_address) (ptid_t ptid, CORE_ADDR *address);
> +
>    int (*supports_non_stop) (void);

and:

>  static struct target_ops win32_target_ops = {
>    win32_create_inferior,
>    win32_attach,
> @@ -1782,6 +1799,9 @@ static struct target_ops win32_target_op
>  #else
>    hostio_last_error_from_errno,
>  #endif
> +  NULL,
> +  NULL,
> +  win32_get_tib_address,
>  };

Please remember to update this correctly, by putting your
new callback as _last_ callback in the structure.  Otherwise,
you'd have to update all other target_ops (linux-low.c, nto-low.c,
spu.low.c).

Otherwise, this version looks good to me.  Please check it in if
Eli is okay with it as well.

-- 
Pedro Alves