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

Luis Machado
2017-02-01T18:54:21Z

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.