[Pdbg] [PATCH v2 15/22] libpdbg: Return status from pdbg_targets_init()

[Amitay Isaacs]

On Mon, 2019-09-23 at 13:49 +1000, Alistair Popple wrote:
> Looks good. Thoughts on adding __attribute__((warn_unused_result)) so
> that 
> other library users will catch this change?

This is really in prepration to getting rid of all the asserts which we
are using as shortcut to error handling (asserts that check for libpdbg
consistency are fine).  I am not sure what's the best way to inform
users of such changes.

I would definitely like to generate warnings for unused results from
all the public api functions.  Is there an easier way of doing that
rather than adding decorations to each function declaration? 

> 
> Reviewed-by: Alistair Popple <alistair at popple.id.au>
> 
> On Friday, 20 September 2019 3:16:44 PM AEST Amitay Isaacs wrote:
> > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > ---
> >  libpdbg/device.c                | 8 +++++---
> >  libpdbg/libpdbg.h               | 2 +-
> >  src/main.c                      | 3 ++-
> >  src/tests/libpdbg_dtree_test.c  | 2 +-
> >  src/tests/libpdbg_probe_test.c  | 6 +++---
> >  src/tests/libpdbg_target_test.c | 2 +-
> >  6 files changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > index 0742c2b..714b280 100644
> > --- a/libpdbg/device.c
> > +++ b/libpdbg/device.c
> > @@ -731,23 +731,25 @@ skip:
> >  	}
> >  }
> >  
> > -void pdbg_targets_init(void *fdt)
> > +bool pdbg_targets_init(void *fdt)
> >  {
> >  	if (!fdt)
> >  		fdt = pdbg_default_dtb();
> >  
> >  	if (!fdt) {
> >  		pdbg_log(PDBG_ERROR, "Could not find a system device
> > tree\n");
> > -		return;
> > +		return false;
> >  	}
> >  
> >  	/* Root node needs to be valid when this function returns */
> >  	pdbg_dt_root = dt_new_node("", NULL, 0);
> > -	assert(pdbg_dt_root);
> > +	if (!pdbg_dt_root)
> > +		return false;
> >  
> >  	dt_expand(pdbg_dt_root, fdt);
> >  
> >  	pdbg_targets_init_virtual(pdbg_dt_root, pdbg_dt_root);
> > +	return true;
> >  }
> >  
> >  char *pdbg_target_path(struct pdbg_target *target)
> > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> > index de3fc17..31a644e 100644
> > --- a/libpdbg/libpdbg.h
> > +++ b/libpdbg/libpdbg.h
> > @@ -101,7 +101,7 @@ uint64_t pdbg_target_address(struct pdbg_target
> > *target, 
> uint64_t *size);
> >  	(index == 0 ? pdbg_target_address(target, size) : assert(0))
> >  
> >  /* Misc. */
> > -void pdbg_targets_init(void *fdt);
> > +bool pdbg_targets_init(void *fdt);
> >  void pdbg_target_probe_all(struct pdbg_target *parent);
> >  enum pdbg_target_status pdbg_target_probe(struct pdbg_target
> > *target);
> >  void pdbg_target_release(struct pdbg_target *target);
> > diff --git a/src/main.c b/src/main.c
> > index f7f891a..a508034 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -580,7 +580,8 @@ int main(int argc, char *argv[])
> >  	if (backend)
> >  		pdbg_set_backend(backend, device_node);
> >  
> > -	pdbg_targets_init(NULL);
> > +	if (!pdbg_targets_init(NULL))
> > +		return 1;
> >  
> >  	if (pathsel_count) {
> >  		if (!path_target_parse(pathsel, pathsel_count))
> > diff --git a/src/tests/libpdbg_dtree_test.c
> > b/src/tests/libpdbg_dtree_test.c
> > index f6d4cbf..d3c4a89 100644
> > --- a/src/tests/libpdbg_dtree_test.c
> > +++ b/src/tests/libpdbg_dtree_test.c
> > @@ -101,7 +101,7 @@ int main(int argc, const char **argv)
> >  		usage();
> >  	}
> >  
> > -	pdbg_targets_init(NULL);
> > +	assert(pdbg_targets_init(NULL));
> >  
> >  	target = pdbg_target_from_path(NULL, argv[3]);
> >  	if (!target)
> > diff --git a/src/tests/libpdbg_probe_test.c
> > b/src/tests/libpdbg_probe_test.c
> > index ed8a800..fba7a23 100644
> > --- a/src/tests/libpdbg_probe_test.c
> > +++ b/src/tests/libpdbg_probe_test.c
> > @@ -74,7 +74,7 @@ static void test1(void)
> >  	struct pdbg_target *root, *target;
> >  
> >  	pdbg_set_backend(PDBG_BACKEND_FAKE, NULL);
> > -	pdbg_targets_init(NULL);
> > +	assert(pdbg_targets_init(NULL));
> >  
> >  	root = pdbg_target_root();
> >  	assert(root);
> > @@ -122,7 +122,7 @@ static void test2(void)
> >  	enum pdbg_target_status status;
> >  
> >  	pdbg_set_backend(PDBG_BACKEND_FAKE, NULL);
> > -	pdbg_targets_init(NULL);
> > +	assert(pdbg_targets_init(NULL));
> >  
> >  	root = pdbg_target_root();
> >  	assert(root);
> > @@ -199,7 +199,7 @@ static void test3(void)
> >  	enum pdbg_target_status status;
> >  
> >  	pdbg_set_backend(PDBG_BACKEND_FAKE, NULL);
> > -	pdbg_targets_init(NULL);
> > +	assert(pdbg_targets_init(NULL));
> >  
> >  	root = pdbg_target_root();
> >  	assert(root);
> > diff --git a/src/tests/libpdbg_target_test.c b/src/tests/
> libpdbg_target_test.c
> > index 9806281..c68590f 100644
> > --- a/src/tests/libpdbg_target_test.c
> > +++ b/src/tests/libpdbg_target_test.c
> > @@ -66,7 +66,7 @@ int main(void)
> >  	int count, i;
> >  
> >  	pdbg_set_backend(PDBG_BACKEND_FAKE, NULL);
> > -	pdbg_targets_init(NULL);
> > +	assert(pdbg_targets_init(NULL));
> >  
> >  	root = pdbg_target_root();
> >  	assert(root);
> > 
> 
> 
> 

Amitay.
-- 

Challenges can be stepping stones or stumbling blocks. It's just a matter of
how you view them.