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

Samuel Mendoza-Jonas sam at mendozajonas.com
Wed Mar 7 13:49:04 AEDT 2018


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.

> 
> >  
> >  	/* 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



More information about the Petitboot mailing list