[Pdbg] [PATCH v2 13/16] libpdbg: Remove old dt_prop functions

[Alistair Popple]

On Wednesday, 7 November 2018 5:52:51 PM AEDT Amitay Isaacs wrote:
> If a function returns 0/-1 for success/failure, I think returning bool
> makes code easier to read.  I guess it's just the matter of style.

Fair comment and I think we probably have a little inconsistency in return 
codes.  However I was trying to limit the scope of these cleanups to removing 
dead code and making non-API functions static were possible to avoid conflicts 
when using it in other projects, so hopefully I'm not actually adding/changing 
any return codes here?

<snip>
 
> > 
> > +int pdbg_target_u32_index(struct pdbg_target *target, const char
> > *name, int index, uint32_t *val)
> > +{
> > +        size_t len;
> > +        void *p;
> > +
> > +        p = pdbg_get_target_property(target, name, &len);
> > +        if (!p)
> > +                return -1;
> > +
> > +        assert(len >= (index+1)*sizeof(u32));
> 
> May be replace u32 with uint32_t?

Sure.

> > +
> > +        /* Always aligned, so this works. */
> > +        *val = be32toh(((const u32 *)p)[index]);
> 
> Why do we need const cast here?

This is copied code but a good question. I guess anything coming from the DT 
could come from read-only memory and is therefore strictly const however I'm 
not it matters here.

> Can p be defined as uint32_t *?

Yes :-)

I guess the comment on alignment still standards, and we should probably 
actually assert that it is true as it could lead to some pretty "fun" bugs if 
for some reason it isn't.

> > +        return 0;
> > +}
> > +
> > 
> >  uint32_t pdbg_target_chip_id(struct pdbg_target *target)
> >  {
> >  
> >          uint32_t id;
> > 
> > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> > index f8fe3e8..6108af9 100644
> > --- a/libpdbg/libpdbg.h
> > +++ b/libpdbg/libpdbg.h
> > @@ -73,6 +73,7 @@ void pdbg_target_set_property(struct pdbg_target
> > *target, const char *name, cons
> > 
> >  /* Get the given property and return the size */
> >  void *pdbg_target_property(struct pdbg_target *target, const char
> > 
> > *name, size_t *size);
> > 
> >  int pdbg_target_u32_property(struct pdbg_target *target, const char
> > 
> > *name, uint32_t *val);
> > +int pdbg_target_u32_index(struct pdbg_target *target, const char
> > *name, int index, uint32_t *val);
> > 
> >  uint64_t pdbg_target_address(struct pdbg_target *target, uint64_t
> > 
> > *size);
> > 
> >  /* Old deprecated for names for the above. Do not use for new
> > 
> > projects
> > diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c
> > index 2411103..9d43725 100644
> > --- a/libpdbg/p9chip.c
> > +++ b/libpdbg/p9chip.c
> > @@ -118,8 +118,10 @@ static struct thread_state
> > p9_get_thread_status(struct thread *thread)
> > 
> >  static int p9_thread_probe(struct pdbg_target *target)
> >  {
> >  
> >  	struct thread *thread = target_to_thread(target);
> > 
> > +	uint32_t tid;
> > 
> > -	thread->id = dt_prop_get_u32(target, "tid");
> > +	assert(!pdbg_target_u32_property(target, "tid", &tid));
> > +	thread->id = tid;
> > 
> >  	thread->status = p9_get_thread_status(thread);
> >  	
> >  	return 0;
> 
> Amitay.