linux-kernel - Re: [Outreachy kernel] [PATCH] staging: emxx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4bd376a6db77ef779972f377cc2c3789d03668e7.camel@gmail.com>
Date:   Thu, 02 Apr 2020 18:42:44 -0700
From:   "John B. Wyatt IV" <jbwyatt4@...il.com>
To:     Stefano Brivio <sbrivio@...hat.com>
Cc:     outreachy-kernel@...glegroups.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Saiyam Doshi <saiyamdoshi.in@...il.com>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code

On Fri, 2020-04-03 at 01:50 +0200, Stefano Brivio wrote:
> On Wed,  1 Apr 2020 19:17:06 -0700
> "John B. Wyatt IV" <jbwyatt4@...il.com> wrote:
> 
> > Remove unused code surrounded by an #if 0 block.
> > 
> > Code has not been altered since 2014 as reported by git blame.
> > 
> > Reported by checkpatch.
> > 
> > Signed-off-by: John B. Wyatt IV <jbwyatt4@...il.com>
> > ---
> >  drivers/staging/emxx_udc/emxx_udc.h | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/drivers/staging/emxx_udc/emxx_udc.h
> > b/drivers/staging/emxx_udc/emxx_udc.h
> > index 9c2671cb32f7..bbfebe331033 100644
> > --- a/drivers/staging/emxx_udc/emxx_udc.h
> > +++ b/drivers/staging/emxx_udc/emxx_udc.h
> > @@ -9,12 +9,6 @@
> >  #define _LINUX_EMXX_H
> >  
> >  /*--------------------------------------------------------------
> > -------------*/
> > -/*----------------- Default undef */
> > -#if 0
> > -#define DEBUG
> > -#define UDC_DEBUG_DUMP
> > -#endif
> > -
> >  /*----------------- Default define */
> >  #define	USE_DMA	1
> >  #define USE_SUSPEND_WAIT	1
> 
> Formally, this is fine. But... think about it: this driver might be
> rather buggy, so the first thing one might want to do with it is to
> "enable" those two defines.
> 
> In general, that stuff has to disappear, and proper debugging
> facilities have to be used, but with a driver in this state, as long
> as
> proper debugging facilities aren't there, you might be doing more
> harm
> than good.

DEBUG is not actually used as far as I can tell (I am still new to
kernel debugging systems to please correct me). There is only a pair of
.c and .h files for this small driver.

UDC_DEBUG_DUMP is only used twice in the entire kernel-both for if
statements.

Should we just set it to:

#define UDC_DEBUG_DUMP 0

And leave the other 3 lines out? Please let me know for a v2.

> 
> -- 
> Stefano
>