diff mbox series

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

Message ID 20190920051651.9620-20-amitay@ozlabs.org
State Superseded
Headers show
Series Add system device tree to libpdbg | expand

Commit Message

Amitay Isaacs Sept. 20, 2019, 5:16 a.m. UTC 
Signed-off-by: Amitay Isaacs <amitay@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(-)

Comments

Alistair Popple Sept. 23, 2019, 3:49 a.m. UTC | #1 
Looks good. Thoughts on adding __attribute__((warn_unused_result)) so that 
other library users will catch this change?

Reviewed-by: Alistair Popple <alistair@popple.id.au>

On Friday, 20 September 2019 3:16:44 PM AEST Amitay Isaacs wrote:
> Signed-off-by: Amitay Isaacs <amitay@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 Isaacs Sept. 23, 2019, 8:21 a.m. UTC | #2 
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@popple.id.au>
> 
> On Friday, 20 September 2019 3:16:44 PM AEST Amitay Isaacs wrote:
> > Signed-off-by: Amitay Isaacs <amitay@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.
diff mbox series

Patch

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);