diff mbox series

[2/3] net: Expose some errors generated in net_init

Message ID 20200912214544.362594-3-seanga2@gmail.com
State Accepted
Delegated to: Tom Rini
Headers show
Series log: Fix segfault in sandbox when LOG_LEVEL_DEFAULT >= LOGL_DEBUG | expand

Commit Message

Sean Anderson Sept. 12, 2020, 9:45 p.m. UTC
net_init does not always succeed, and there is no existing mechanism to
discover errors. This patch allows callers of net_init (such as net_init)
to handle errors. The root issue is that eth_get_dev can fail, but
net_init_loop doesn't expose that. The ideal way to fix eth_get_dev would
be to return an error with ERR_PTR, but there are a lot of callers, and all
of them just check if it's NULL. Another approach would be to change the
signature to something like

int eth_get_dev(struct udevice **pdev)

but that would require rewriting all of the many callers.

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

 include/net.h    |  2 +-
 net/eth-uclass.c |  3 +++
 net/net.c        | 15 +++++++++++----
 3 files changed, 15 insertions(+), 5 deletions(-)

Comments

Simon Glass Sept. 17, 2020, 1:09 a.m. UTC | #1
On Sat, 12 Sep 2020 at 15:46, Sean Anderson <seanga2@gmail.com> wrote:
>
> net_init does not always succeed, and there is no existing mechanism to
> discover errors. This patch allows callers of net_init (such as net_init)
> to handle errors. The root issue is that eth_get_dev can fail, but
> net_init_loop doesn't expose that. The ideal way to fix eth_get_dev would
> be to return an error with ERR_PTR, but there are a lot of callers, and all
> of them just check if it's NULL. Another approach would be to change the
> signature to something like
>
> int eth_get_dev(struct udevice **pdev)
>
> but that would require rewriting all of the many callers.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  include/net.h    |  2 +-
>  net/eth-uclass.c |  3 +++
>  net/net.c        | 15 +++++++++++----
>  3 files changed, 15 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini Oct. 12, 2020, 1:15 a.m. UTC | #2
On Sat, Sep 12, 2020 at 05:45:43PM -0400, Sean Anderson wrote:

> net_init does not always succeed, and there is no existing mechanism to
> discover errors. This patch allows callers of net_init (such as net_init)
> to handle errors. The root issue is that eth_get_dev can fail, but
> net_init_loop doesn't expose that. The ideal way to fix eth_get_dev would
> be to return an error with ERR_PTR, but there are a lot of callers, and all
> of them just check if it's NULL. Another approach would be to change the
> signature to something like
> 
> int eth_get_dev(struct udevice **pdev)
> 
> but that would require rewriting all of the many callers.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/include/net.h b/include/net.h
index 1bf9867f8c..b80b13ae7c 100644
--- a/include/net.h
+++ b/include/net.h
@@ -593,7 +593,7 @@  extern int net_ntp_time_offset;			/* offset time from UTC */
 #endif
 
 /* Initialize the network adapter */
-void net_init(void);
+int net_init(void);
 int net_loop(enum proto_t);
 
 /* Load failed.	 Start again. */
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 0d9b75a9a2..1f59d24f54 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -75,6 +75,9 @@  struct udevice *eth_get_dev(void)
 	struct eth_uclass_priv *uc_priv;
 
 	uc_priv = eth_get_uclass_priv();
+	if (!uc_priv)
+		return NULL;
+
 	if (!uc_priv->current)
 		eth_errno = uclass_first_device(UCLASS_ETH,
 				    &uc_priv->current);
diff --git a/net/net.c b/net/net.c
index 28d9eebf9d..3afd0d6fb0 100644
--- a/net/net.c
+++ b/net/net.c
@@ -353,12 +353,19 @@  void net_auto_load(void)
 	tftp_start(TFTPGET);
 }
 
-static void net_init_loop(void)
+static int net_init_loop(void)
 {
 	if (eth_get_dev())
 		memcpy(net_ethaddr, eth_get_ethaddr(), 6);
+	else
+		/*
+		 * Not ideal, but there's no way to get the actual error, and I
+		 * don't feel like fixing all the users of eth_get_dev to deal
+		 * with errors.
+		 */
+		return -ENONET;
 
-	return;
+	return 0;
 }
 
 static void net_clear_handlers(void)
@@ -373,7 +380,7 @@  static void net_cleanup_loop(void)
 	net_clear_handlers();
 }
 
-void net_init(void)
+int net_init(void)
 {
 	static int first_call = 1;
 
@@ -396,7 +403,7 @@  void net_init(void)
 		first_call = 0;
 	}
 
-	net_init_loop();
+	return net_init_loop();
 }
 
 /**********************************************************************/