[Pdbg] [PATCH v4 09/30] libpdbg: Add a function to map real target to virtual

[Amitay Isaacs]

On Wed, 2019-10-09 at 13:53 +1100, Alistair Popple wrote:
> On Wednesday, 9 October 2019 1:40:20 PM AEDT Amitay Isaacs wrote:
> > On Wed, 2019-10-09 at 13:33 +1100, Alistair Popple wrote:
> > > Just a quick question. Can we assume:
> > > 
> > > target_is_virtual(target_to_virtual(target)) will always be true?
> > 
> > No. That's not strictly correct.  target_to_virtual() will only map
> > to
> > virtual targets, if there is a linked virtual target.  Otherwise it
> > will return the same target, as there is nothing to map.
> > 
> > 
> > > And that:
> > > 
> > > target_is_virtual(target_to_real(target)) will always be false?
> > 
> > Similar argument as above.  Some virtual nodes may not have any
> > linked
> > real nodes.  In that case, target_to_real() will return the same
> > virtual target. (For example, "/proc0" is a virtual target without
> > any
> > real target linked to it.)
> > 
> 
> So in these cases are we asserting that if we fail to map a node to
> a 
> [virtual|real] node that it is still correct to continue on treating
> the 
> original node as if it were of the requested type?


Well the motivation behind target_to_real() and target_to_virtual() is
adding convinience functions that help improve the readability of the
traverse code where they are used.  As I described they are not
strictly converting to real/virtual targets, but if possible map it to
the right target.

I can add some comments for these functions if that would help. 

> 
> > > Do you think it be would be worthwhile adding some asserts to
> > > check
> > > this or 
> > > does the probing code (which I'm yet to read) somehow guarantee
> > > those 
> > > assertions are always correct?
> > 
> > I think the tests which focus on the traverse will ensure the
> > correctness of map functions.  We don't specifically need testing
> > for
> > the lower level functions.  Also, these functions are internal to
> > libpdbg and I would have to jump through hoops to test them. :-
> > )  But
> > not impossible to add tests if you strongly feel about it.
> 
> It wasn't so much for testing but rather to ensure we crash and burn
> as early 
> as possible if something unexpected (ie. buggy) is observed. However
> it seems 
> my original assertions were wrong anyway.

I have added assertions where we expect a virtual target.
> 
> - Alistair
> 
> > > - Alistair
> > > 
> > > On Thursday, 3 October 2019 2:18:48 PM AEDT Amitay Isaacs wrote:
> > > > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > > > ---
> > > >  libpdbg/target.c | 8 ++++++++
> > > >  libpdbg/target.h | 1 +
> > > >  2 files changed, 9 insertions(+)
> > > > 
> > > > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > > > index 20f292f..f3d4db8 100644
> > > > --- a/libpdbg/target.c
> > > > +++ b/libpdbg/target.c
> > > > @@ -481,3 +481,11 @@ struct pdbg_target *target_to_real(struct
> > > > pdbg_target 
> > > *target)
> > > >  
> > > >  	return target;
> > > >  }
> > > > +
> > > > +struct pdbg_target *target_to_virtual(struct pdbg_target
> > > > *target)
> > > > +{
> > > > +	if (target->compatible && target->vnode)
> > > > +		return target->vnode;
> > > > +
> > > > +	return target;
> > > > +}
> > > > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > > > index 5d04117..8148f83 100644
> > > > --- a/libpdbg/target.h
> > > > +++ b/libpdbg/target.h
> > > > @@ -66,5 +66,6 @@ const char *pdbg_get_backend_option(void);
> > > >  struct sbefifo *pib_to_sbefifo(struct pdbg_target *target);
> > > >  bool target_is_virtual(struct pdbg_target *target);
> > > >  struct pdbg_target *target_to_real(struct pdbg_target
> > > > *target);
> > > > +struct pdbg_target *target_to_virtual(struct pdbg_target
> > > > *target);
> > > >  
> > > >  #endif
> > > > 
> > > 
> > > 
> > 
> > Amitay.
> > 
> 
> 
> 

Amitay.
-- 

Problems are opportunities in work clothes. - Henry Kaiser