public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Sean Anderson <seanga2@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH 1/3] dev: Disambiguate errors in uclass_find
Date: Sat, 12 Sep 2020 17:45:42 -0400	[thread overview]
Message-ID: <20200912214544.362594-2-seanga2@gmail.com> (raw)
In-Reply-To: <20200912214544.362594-1-seanga2@gmail.com>

There are two cases where uclass_find can return an error. The second is
when the uclass has not yet been init'd. The first is when the driver model
has not been init'd (or has been only partially init'd) and there is no
root uclass driver.

If LOG_SYSLOG is enabled and LOG_DEFAULT_LEVEL is set to LOGL_DEBUG or
higher, log_syslog_emit will try to call net_init before initf_dm has been
called. This in turn (eventually) calls uclass_get for UCLASS_ETH.

If the second error occurs, uclass_get should call uclass_add to create the
uclass. If the first error occurs, then uclass_get should not call
uclass_add (because the uclass list has not been initialized). However, the
first error is expected when calling uclass_get for UCLASS_ROOT, so we add
an exception.

There are several alternative approaches to this patch. One option would be
to duplicate the check against gd->dm_root in uclass_get and not change the
behavior of uclass_find. I did not choose this approach because I think it
it is less clear than the patch as-is. However, that is certainly
subjective, and there is no other technical reason to do it this way.

Another approach would have been to change log_syslog_emit to abort if it
is called too early. I did not choose this approach because the check in
uclass_find to see if gd->dm_root exists implies that functions in its
file are allowed to be called at any time. There is an argument to be made
that callers should ensure that they don't call certain functions too
early. I think it is better to code defensively in these circumstances to
make it easier to discover such errors.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/core/uclass.c | 16 +++++++++++++++-
 test/dm/core.c        |  2 +-
 test/dm/test-main.c   |  2 +-
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c3f1b73cd6..2e098034c9 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -19,6 +19,7 @@
 #include <dm/uclass.h>
 #include <dm/uclass-internal.h>
 #include <dm/util.h>
+#include <linux/err.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -27,7 +28,7 @@ struct uclass *uclass_find(enum uclass_id key)
 	struct uclass *uc;
 
 	if (!gd->dm_root)
-		return NULL;
+		return ERR_PTR(-EAGAIN);
 	/*
 	 * TODO(sjg at chromium.org): Optimise this, perhaps moving the found
 	 * node to the start of the list, or creating a linear array mapping
@@ -144,6 +145,19 @@ int uclass_get(enum uclass_id id, struct uclass **ucp)
 
 	*ucp = NULL;
 	uc = uclass_find(id);
+	if (IS_ERR(uc)) {
+		/*
+		 * If we're getting the root uclass, then uclass_find will fail
+		 * with -EAGAIN (since it thinks there's no uclass list to
+		 * search), but we need to add it anyway (since otherwise it
+		 * would never be created).
+		 */
+		if (PTR_ERR(uc) == -EAGAIN && id == UCLASS_ROOT)
+			uc = NULL;
+		else
+			return PTR_ERR(uc);
+	}
+
 	if (!uc)
 		return uclass_add(id, ucp);
 	*ucp = uc;
diff --git a/test/dm/core.c b/test/dm/core.c
index 8ed5bf7370..a4e0ac06f9 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -733,7 +733,7 @@ static int dm_test_uclass_before_ready(struct unit_test_state *uts)
 	gd->dm_root_f = NULL;
 	memset(&gd->uclass_root, '\0', sizeof(gd->uclass_root));
 
-	ut_asserteq_ptr(NULL, uclass_find(UCLASS_TEST));
+	ut_asserteq_ptr(ERR_PTR(-EAGAIN), uclass_find(UCLASS_TEST));
 
 	return 0;
 }
diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index 38b7b1481a..6ff07a5d32 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -70,7 +70,7 @@ static int dm_test_destroy(struct unit_test_state *uts)
 		 * check that here before we call uclass_find_device().
 		 */
 		uc = uclass_find(id);
-		if (!uc)
+		if (IS_ERR_OR_NULL(uc))
 			continue;
 		ut_assertok(uclass_destroy(uc));
 	}
-- 
2.28.0

  reply	other threads:[~2020-09-12 21:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-12 21:45 [PATCH 0/3] log: Fix segfault in sandbox when LOG_LEVEL_DEFAULT >= LOGL_DEBUG Sean Anderson
2020-09-12 21:45 ` Sean Anderson [this message]
2020-09-17  1:09   ` [PATCH 1/3] dev: Disambiguate errors in uclass_find Simon Glass
2020-09-17  1:44     ` Sean Anderson
2020-09-17  3:44       ` Simon Glass
2020-09-12 21:45 ` [PATCH 2/3] net: Expose some errors generated in net_init Sean Anderson
2020-09-17  1:09   ` Simon Glass
2020-10-12  1:15   ` Tom Rini
2020-09-12 21:45 ` [PATCH 3/3] log: syslog: Handle errors " Sean Anderson
2020-09-17  1:09   ` Simon Glass
2020-10-12  1:15   ` Tom Rini
2020-09-13 20:13 ` [PATCH 0/3] log: Fix segfault in sandbox when LOG_LEVEL_DEFAULT >= LOGL_DEBUG Sean Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200912214544.362594-2-seanga2@gmail.com \
    --to=seanga2@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox