Re: [PATCH RFA] process/thread/lwp identifier mega-patch

[Kevin Buettner <kevinb at cygnus dot com>]

On Feb 16,  5:14pm, Andrew Cagney wrote:

> > E.g, in infrun.c, we have the following declaration:
> > 
> >         static int static int previous_inferior_pid;
> 
> ARRG!
> 
> > My patches change this declaration to:
> > 
> >         static struct ptid *previous_inferior_ptid;
> > 
> > We would need to make sure this (and other static globals) are
> > reinitialized when the thread list is wiped out.
> 
> Really nasty would be to enter each of those globals into a database and
> trash them at the same time as the thread pool is trashed.

That's what I had in mind.  I'd make the _initialize_* functions
responsible for registering the various static globals in a simple
database (probably just a linked list).

> It might even be a tolerable workaround since those globals will
> eventually need to be deleted.

Right.

I seem to recall that there were some other data structures which
might need to be reinitialized as well.  (The thread list comes
to mind; OTOH, since we're wiping the threads anyway, this might
not be a problem.  But I think there might've been others as well.
I'll need to revisit the code to be sure.)

> > Another alternative is to make the execution context identifiers (or
> > ECIs for short) ``struct ptid'' instead of ``struct ptid *''.  I.e,
> > make the ECI a struct instead of a pointer to a struct.  The problem
> > with doing this is that the ECI's type can no longer be opaque.
> 
> Again as an imtermediate step yes.

Hmmm... in some respects, I really prefer this route.  Now if I
could just get you to agree to using a typedef, I could do the
following:

    struct ptid			/* Alas, not opaque... */
      {
        ...
      };
    typedef struct ptid ptid;

The code would then be changed to use ``ptid'' everywhere that
``struct ptid *'' currently appears (in my patch).

Later on, when we're ready to move to using a pointer to a struct,
we'll be able to use something along the following lines:

    struct ptid;		/* Now struct ptid is opaque */
    typedef struct ptid *ptid;

The nice thing about this is that very little other code would need
to change.  (Just the accessors and constructors.)

But I seem to recall that you had a problem with typedef...

> > One can argue that if GDB accesses a defunct ECI (regardless of
> > implementation) at all, it is behaving incorrectly, because this
> > behavior is wrong regardless of whether the ECI is a struct or a
> > dangling pointer.  It's just that it could be catastrophic if it's the
> > latter...
> > 
> > So maybe it'd be best if we make sure that each and every ECI
> > occurence in the code is initialized properly when the thread list is
> > cleaned up.  (In other words, I'm coming around to liking your
> > suggestion again...)
> 
> You should probably look carefully at
> http://sources.redhat.com/ml/gdb/2001-02/msg00210.html .  In that
> diagram, ``context'' roughly correspond to ``struct ptid *''.

Sort of.  I have the feeling that I'm just quibbling about
terminology, but at the moment I would call ``struct ptid *'' a
context identifier since it contains nothing more than the identifiers
which may be used to refer to a context.  It is certainly the case
that we could add members to struct ptid (or maybe just use struct
thread_info) to (more) fully represent a context.

Kevin