Re: [PATCH v2 4/6] Share parts of gdb/gdbthread.h with gdbserver

[Luis Machado <lgustavo@codesourcery.com>]

On 01/18/2017 09:36 AM, Sergio Durigan Junior wrote:
> Again, it was necessary to share a few functions declared on
> gdb/gdbthread.h with gdbserver, because they are needed by
> fork_inferior.  I decided to implement them on
> gdb/gdbserver/inferiors.c because that's where the thread functions
> are also implemented on gdbserver.  Some of these functions do not
> need to be implemented on gdbserver, or don't make sense there, so
> they are left blank and commented properly.
>
> gdb/ChangeLog:
> 2017-01-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* Makefile.in (HFILES_NO_SRCDIR): Add "common/common-gdbthread.h".
> 	* common/common-gdbthread.h: New file, with parts from
> 	"gdb/gdbthread.h".
> 	* fork-child.c (fork_inferior): Update call of "set_executing".
> 	* gdbthread.h: Include "common-gdbthread.h".
> 	(init_thread_list): Moved to "common/common-gdbthread.h".
> 	(add_thread_silent): Likewise.
> 	(switch_to_thread): Likewise.
> 	(set_executing): Likewise.
> 	* thread.c (set_executing): Update function comment.
>
> gdb/gdbserver/ChangeLog:
> 2017-01-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* inferiors.c (init_thread_list): New function.
> 	(switch_to_thread): Likewise.
> 	(set_executing): Likewise.
> 	(add_thread_silent): Likewise.
> 	* server.c (get_last_target_waitstatus): Likewise.
> 	* server.h (get_last_target_waitstatus): Likewise.
> ---
>  gdb/Makefile.in               |  1 +
>  gdb/common/common-gdbthread.h | 45 +++++++++++++++++++++++++++++++++++++++++++
>  gdb/gdbserver/inferiors.c     | 41 +++++++++++++++++++++++++++++++++++++++
>  gdb/gdbserver/server.c        |  8 ++++++++
>  gdb/gdbserver/server.h        |  3 +++
>  gdb/gdbthread.h               | 20 +------------------
>  gdb/thread.c                  |  2 ++
>  7 files changed, 101 insertions(+), 19 deletions(-)
>  create mode 100644 gdb/common/common-gdbthread.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index c0325d5..3e49e6e 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1467,6 +1467,7 @@ HFILES_NO_SRCDIR = \
>  	common/common-debug.h \
>  	common/common-defs.h \
>  	common/common-exceptions.h \
> +	common/common-gdbthread.h \
>  	common/common-regcache.h \
>  	common/common-types.h \
>  	common/common-utils.h \
> diff --git a/gdb/common/common-gdbthread.h b/gdb/common/common-gdbthread.h
> new file mode 100644
> index 0000000..eb66de9
> --- /dev/null
> +++ b/gdb/common/common-gdbthread.h
> @@ -0,0 +1,45 @@
> +/* Common multi-process/thread control defs for GDB and gdbserver.
> +   Copyright (C) 1987-2017 Free Software Foundation, Inc.
> +   Contributed by Lynx Real-Time Systems, Inc.  Los Gatos, CA.
> +
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef COMMON_THREAD_H
> +#define COMMON_THREAD_H
> +
> +struct target_waitstatus;
> +
> +/* Create an empty thread list, or empty the existing one.  */
> +extern void init_thread_list (void);
> +
> +/* Switch from one thread to another.  */
> +extern void switch_to_thread (ptid_t ptid);
> +
> +/* Marks thread PTID as executing, or not.  If PTID is minus_one_ptid,
> +   marks all threads.
> +
> +   Note that this is different from the running state.  See the
> +   description of state and executing fields of struct
> +   thread_info.  */
> +extern void set_executing (ptid_t ptid, int executing);
> +
> +/* Add a thread to the thread list and return the pointer to the new
> +   thread.  Caller may use this pointer to initialize the private
> +   thread data.  */
> +extern struct thread_info *add_thread_silent (ptid_t ptid);
> +
> +#endif /* ! COMMON_THREAD_H */
> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
> index b65a726..441ec2c 100644
> --- a/gdb/gdbserver/inferiors.c
> +++ b/gdb/gdbserver/inferiors.c
> @@ -468,3 +468,44 @@ make_cleanup_restore_current_thread (void)
>  {
>    return make_cleanup (do_restore_current_thread_cleanup, current_thread);
>  }
> +
> +/* See common/common-gdbthread.h.  */
> +
> +void
> +init_thread_list (void)
> +{
> +  /* To be implemented.  */
> +}
> +
> +/* See common/common-gdbthread.h.  */
> +
> +void
> +switch_to_thread (ptid_t ptid)
> +{
> +  if (!ptid_equal (ptid, minus_one_ptid))
> +    current_thread = find_thread_ptid (ptid);
> +}
> +
> +/* See common/common-gdbthread.h.  */
> +
> +void
> +set_executing (ptid_t ptid ATTRIBUTE_UNUSED, int executing ATTRIBUTE_UNUSED)
> +{
> +  gdb_assert (current_thread != NULL);
> +  current_thread->last_resume_kind = resume_stop;
> +  current_thread->last_status = get_last_target_waitstatus ();
> +}
> +


I'm still not sure about this particular function. But i'd like to hear 
what other think.

Otherwise i have no further comments on this one.