On Thu, Jan 05, 2023 at 12:37:58PM +0100, Patrick DELAUNAY wrote:
> Hi Tom,
>
> On 1/3/23 21:35, Tom Rini wrote:
> > On Thu, Dec 15, 2022 at 10:15:50AM +0100, Patrick Delaunay wrote:
> > > Much of the fastboot code predates the introduction of Kconfig and
> > > has quite a few #ifdefs in it which is unnecessary now that we can use
> > > IS_ENABLED() et al.
> > >
> > > Signed-off-by: Patrick Delaunay <patrick.delau...@foss.st.com>
> > > ---
> > >
> > > cmd/fastboot.c | 35 +++++------
> > > drivers/fastboot/fb_command.c | 104 ++++++++++++--------------------
> > > drivers/fastboot/fb_common.c | 11 ++--
> > > drivers/fastboot/fb_getvar.c | 49 ++++++---------
> > > drivers/usb/gadget/f_fastboot.c | 7 +--
> > > include/fastboot.h | 13 ----
> > > net/fastboot.c | 8 +--
> > > 7 files changed, 82 insertions(+), 145 deletions(-)
> > >
> > > diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> > > index b498e4b22bb3..b94dbd548843 100644
> > > --- a/cmd/fastboot.c
> > > +++ b/cmd/fastboot.c
> > > @@ -19,8 +19,14 @@
> > > static int do_fastboot_udp(int argc, char *const argv[],
> > > uintptr_t buf_addr, size_t buf_size)
> > > {
> > > -#if CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)
> > > - int err = net_loop(FASTBOOT);
> > > + int err;
> > > +
> > > + if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
> > > + pr_err("Fastboot UDP not enabled\n");
> > > + return CMD_RET_FAILURE;
> > > + }
> > > +
> > > + err = net_loop(FASTBOOT);
> > > if (err < 0) {
> > > printf("fastboot udp error: %d\n", err);
> > > @@ -28,21 +34,21 @@ static int do_fastboot_udp(int argc, char *const
> > > argv[],
> > > }
> > > return CMD_RET_SUCCESS;
> > > -#else
> > > - pr_err("Fastboot UDP not enabled\n");
> > > - return CMD_RET_FAILURE;
> > > -#endif
> > > }
> > This probably needs to become an if (CONFIG_IS_ENABLED(...)) { ... }
> > else { ... } in order to remain size-neutral.
>
>
> Are you sure ?
>
>
> {
> if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
> ....
> return CMD_RET_FAILURE;
> }
>
> ....
>
> return CMD_RET_SUCCESS;
> }
>
>
> For me, it is exactly the same size after compiler/linker than :
>
>
> {
> if (!CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)) {
> ....
> return CMD_RET_FAILURE;
> } else {
> ....
> return CMD_RET_SUCCESS;
>
> }
>
> }
>
>
> if UDP_FUNCTION_FASTBOOTis activated or not....
>
> or I forget something during the Christmas break.If you've size-tested and it's the same, OK. I'm just worried about strings not being discarded since that's sometimes an unexpected side effect / bug. -- Tom
signature.asc
Description: PGP signature
