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

[Pedro Alves <palves at redhat dot com>]

On 02/08/2017 03:22 AM, Sergio Durigan Junior wrote:

> 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.
> +   
> +

Spurious blank line.  That "contributed by" line is meaningless
for this new file, please remove it.

> +   This file is part of GDB.

> +#ifndef COMMON_THREAD_H
> +#define COMMON_THREAD_H

This macro name does not patch the file name.

> +/* 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 ();
> +}
> +

This is a bit too hacky to live IMO.  :-/  The implementation
of the function is doing nothing related to its interface.

Do we really need the set_executing call in the new shared file?  AFAICS, 
set_executing is only called at the very end of startup_inferior today.
Couldn't we leave that call out of the common code and add it on the
gdb side, after the common startup_inferior returns?

>  
> -/* 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);
> -
>  /* Reports if thread PTID is executing.  */
>  extern int is_executing (ptid_t ptid);

Having a setter in one place, and the getter somewhere
else is sure to generate confusion.  In cases like these,
please leave a breadcrumb comment.  (there may be more cases.)

Thanks,
Pedro Alves