| Message ID | 20240902125917.119941-1-jerome.forissier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: guard call to tftp_start() with #if defined(CONFIG_CMD_TFTPBOOT) | expand |
Hi Jerome, On 9/2/24 2:59 PM, Jerome Forissier wrote: > net_auto_load() cannot call tftp_start() if CONFIG_CMD_TFTPBOOT is > disabled. > > Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> > --- > net/net.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/net.c b/net/net.c > index d9bc9df643f..35961ba1c7d 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -334,6 +334,7 @@ void net_auto_load(void) > net_set_state(NETLOOP_SUCCESS); > return; > } > +#if defined(CONFIG_CMD_TFTPBOOT) I think we're supposed to use if IS_ENABLED(CONFIG_CMD_TFTPBOOT) now. Additionally, it's highly recommended to rather use "normal" if's wherever possible, i.e.: if (IS_ENABLED(CONFIG_CMD_TFTPBOOT)) { if (net_check_prereq(TFTPGET)) { if (IS_ENABLED(CONFIG_BOOTP_SERVERIP)) { [...] return; } tftp_start(TFTPGET); } } Would that work? Cheers, Quentin
Hi Quentin, On 9/2/24 15:09, Quentin Schulz wrote: > Hi Jerome, > > On 9/2/24 2:59 PM, Jerome Forissier wrote: >> net_auto_load() cannot call tftp_start() if CONFIG_CMD_TFTPBOOT is >> disabled. >> >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> >> --- >> net/net.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/net/net.c b/net/net.c >> index d9bc9df643f..35961ba1c7d 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -334,6 +334,7 @@ void net_auto_load(void) >> net_set_state(NETLOOP_SUCCESS); >> return; >> } >> +#if defined(CONFIG_CMD_TFTPBOOT) > > I think we're supposed to use if IS_ENABLED(CONFIG_CMD_TFTPBOOT) now. Ah yes. I thought it was only with an if () construct but I suppose that works fine with #if too. > Additionally, it's highly recommended to rather use "normal" if's wherever possible, i.e.: > > if (IS_ENABLED(CONFIG_CMD_TFTPBOOT)) { > if (net_check_prereq(TFTPGET)) { > if (IS_ENABLED(CONFIG_BOOTP_SERVERIP)) { > [...] > return; > } > tftp_start(TFTPGET); > } > } > > Would that work? Absolutely. I thought about doing just that but I wrongly assumed tftp_start() would not even be defined as a prototype if CMD_TFTPBOOT was disabled, which is clearly not the case. So yes, that works and I will send a v2. Thanks,
diff --git a/net/net.c b/net/net.c index d9bc9df643f..35961ba1c7d 100644 --- a/net/net.c +++ b/net/net.c @@ -334,6 +334,7 @@ void net_auto_load(void) net_set_state(NETLOOP_SUCCESS); return; } +#if defined(CONFIG_CMD_TFTPBOOT) if (net_check_prereq(TFTPGET)) { /* We aren't expecting to get a serverip, so just accept the assigned IP */ if (IS_ENABLED(CONFIG_BOOTP_SERVERIP)) { @@ -345,6 +346,7 @@ void net_auto_load(void) return; } tftp_start(TFTPGET); +#endif } static int net_init_loop(void)
net_auto_load() cannot call tftp_start() if CONFIG_CMD_TFTPBOOT is disabled. Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> --- net/net.c | 2 ++ 1 file changed, 2 insertions(+)