[PATCH 3/5] discover: Fix unused function warning

[Samuel Mendoza-Jonas]

On Wed, 2018-03-07 at 13:49 +1100, Samuel Mendoza-Jonas wrote:
> On Tue, 2018-03-06 at 16:03 +1100, Cyril Bur wrote:
> > On Tue, 2018-03-06 at 14:32 +1030, Joel Stanley wrote:
> > > clang errors out about an unused have_busybox function:
> > > 
> > >  discover/paths.c:44:13: error: unused function 'have_busybox' [-Werror,-Wunused-function]
> > >  static bool have_busybox(void)
> > >             ^
> > > 
> > > The function is only used once, so move the preprocessor guard down to
> > > the use site instead.
> > 
> > This makes it sound like clang is wrong, its single use is inside
> > #ifndef PETITBOOT_TEST which, I assume you must have had defined.
> > 
> > Might I suggest moving it down ~30 lines into the #ifndef section?
> > 
> > > 
> > > Signed-off-by: Joel Stanley <joel at jms.id.au>
> > > ---
> > > I understand if you don't want to take this one as-is.
> > > 
> > >  discover/paths.c | 13 +++----------
> > >  1 file changed, 3 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/discover/paths.c b/discover/paths.c
> > > index 24e978b4e2c1..1f279a103b44 100644
> > > --- a/discover/paths.c
> > > +++ b/discover/paths.c
> > > @@ -41,15 +41,6 @@ struct load_task {
> > >  	void			*async_data;
> > >  };
> > >  
> > > -static inline bool have_busybox(void)
> > > -{
> > > -#ifdef WITH_BUSYBOX
> > > -	return true;
> > > -#else
> > > -	return false;
> > > -#endif
> > > -}
> > > -
> > >  const char *mount_base(void)
> > >  {
> > >  	return DEVICE_MOUNT_BASE;
> > > @@ -571,8 +562,10 @@ struct load_url_result *load_url_async(void *ctx, struct pb_url *url,
> > >  		task->process->stdout_data = stdout_data;
> > >  	}
> > >  
> > > -	if (!stdout_cb && stdout_data && have_busybox())
> > > +#ifdef WITH_BUSYBOX
> > > +	if (!stdout_cb && stdout_data)
> > >  		task->process->stdout_cb = busybox_progress_cb;
> > > +#endif
> > 
> > Fair warning, doing this gets meeeesssssyyyyyyy. Feel free to have a
> > look at the current and previous versions of
> > arch/powerpc/kernel/process.c in Linux.
> > 
> > The current solution over there is to do
> > #ifdef WITH_BUSYBOX
> > static inline bool have_busybox(void) { return true; }
> > #else
> > static inline bool have_busybox(void) { return false; }
> > #endif
> > 
> > Somewhere at the top of the file or in a header.
> 
> This is probably the way to go - otherwise with the original patch we get
> 
> 13:46:09 discover/paths.c:135:12: warning: ‘busybox_progress_cb’ defined
> but not used [-Wunused-function]
> 13:46:09  static int busybox_progress_cb(void *arg)
> 
> which is no fun.

Or if you have another coffee, technically unrelated and something I
should also fix up :)

> 
> > 
> > >  
> > >  	/* Make sure we save output for any task that has a custom handler */
> > >  	if (task->process->stdout_cb) {
> > 
> > _______________________________________________
> > Petitboot mailing list
> > Petitboot at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/petitboot
> 
> _______________________________________________
> Petitboot mailing list
> Petitboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot