linux-kernel - Re: [PATCH 2/3] drivers/staging/rtl8192u: Remove comment-out code

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]

Date:   Mon, 3 Jun 2019 15:06:37 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Christian Müller <muellerch-privat@....de>
Cc:     devel@...verdev.osuosl.org, felix.trommer@...mail.de,
        linux-kernel@...cs.fau.de, linux-kernel@...r.kernel.org,
        johnfwhitmore@...il.com
Subject: Re: [PATCH 2/3] drivers/staging/rtl8192u: Remove comment-out code

On Mon, Jun 03, 2019 at 02:21:03PM +0200, Christian Müller wrote:
> From: Felix Trommer <felix.trommer@...mail.de>
> 
> Remove the majority of all commented-out code. Commented out code, that
> was adressed by a comment itself is not removed. The code that we left
> in as comments was mostly due to the "pseudo-code"-like nature of the
> code that helped to clarify certain functionalities.
> 
> Signed-off-by: Felix Trommer <felix.trommer@...mail.de>
> Signed-off-by: Christian Müller <muellerch-privat@....de>
> ---
>  drivers/staging/rtl8192u/ieee80211/dot11d.h   |  2 +-
>  .../staging/rtl8192u/ieee80211/ieee80211.h    | 35 ++-----
>  .../rtl8192u/ieee80211/ieee80211_crypt_ccmp.c |  1 -
>  .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c |  1 -
>  .../rtl8192u/ieee80211/ieee80211_module.c     |  2 -
>  .../staging/rtl8192u/ieee80211/ieee80211_rx.c | 50 +---------
>  .../rtl8192u/ieee80211/ieee80211_softmac.c    | 93 +++----------------
>  .../rtl8192u/ieee80211/ieee80211_softmac_wx.c | 13 +--
>  .../staging/rtl8192u/ieee80211/ieee80211_tx.c |  3 +-
>  .../staging/rtl8192u/ieee80211/ieee80211_wx.c | 19 +---
>  .../rtl8192u/ieee80211/rtl819x_BAProc.c       |  3 -
>  .../staging/rtl8192u/ieee80211/rtl819x_HT.h   |  2 -
>  .../rtl8192u/ieee80211/rtl819x_HTProc.c       | 54 ++---------
>  .../staging/rtl8192u/ieee80211/rtl819x_Qos.h  |  4 +-
>  .../rtl8192u/ieee80211/rtl819x_TSProc.c       | 11 ---
>  drivers/staging/rtl8192u/r8190_rtl8256.c      |  3 -
>  drivers/staging/rtl8192u/r8192U_core.c        |  5 +-
>  drivers/staging/rtl8192u/r8192U_dm.c          | 60 ------------
>  drivers/staging/rtl8192u/r8192U_wx.c          |  2 -
>  drivers/staging/rtl8192u/r819xU_firmware.c    |  1 -
>  20 files changed, 38 insertions(+), 326 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.h b/drivers/staging/rtl8192u/ieee80211/dot11d.h
> index 8b485fa18089..f27267307b3f 100644
> --- a/drivers/staging/rtl8192u/ieee80211/dot11d.h
> +++ b/drivers/staging/rtl8192u/ieee80211/dot11d.h
> @@ -54,4 +54,4 @@ void dot11d_scan_complete(struct ieee80211_device *dev);
>  int is_legal_channel(struct ieee80211_device *dev, u8 channel);
>  int to_legal_channel(struct ieee80211_device *dev, u8 channel);
> 
> -#endif /* #ifndef __INC_DOT11D_H */
> +#endif

This type of comment is there for a good reason :(


> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211.h b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
> index 8be8a94a2253..d110e9333799 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211.h
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211.h
> @@ -106,17 +106,13 @@ struct cb_desc {
>  	u8 bRTSUseShortGI:1;
>  	u8 bMulticast:1;
>  	u8 bBroadcast:1;
> -	/* u8 reserved2:2; */
>  	u8 drv_agg_enable:1;
>  	u8 reserved2:1;
> 
>  	/* Tx Desc related element(12-19) */
>  	u8 rata_index;
>  	u8 queue_index;
> -	/* u8 reserved3; */
> -	/* u8 reserved4; */
>  	u16 txbuf_size;
> -	/* u8 reserved5; */
>  	u8 RATRIndex;
>  	u8 reserved6;
>  	u8 reserved7;
> @@ -127,9 +123,6 @@ struct cb_desc {
>  	u8 rts_rate;
>  	u8 ampdu_factor;
>  	u8 ampdu_density;
> -	/* u8 reserved9; */
> -	/* u8 reserved10; */
> -	/* u8 reserved11; */
>  	u8 DrvAggrNum;
>  	u16 pkt_size;
>  	u8 reserved12;
> @@ -883,7 +876,8 @@ enum ieee80211_mfie {
>  	MFIE_TYPE_QOS_PARAMETER = 222,
>  };
> 
> -/* Minimal header; can be used for passing 802.11 frames with sufficient
> +/*
> + * Minimal header; can be used for passing 802.11 frames with sufficient
>   * information to determine what type of underlying data type is actually
>   * stored in the data.
>   */

That change is not removing commented out code :(

Please be a lot more careful as to what you are doing in a patch when
you submit it...

> @@ -1144,7 +1138,6 @@ struct ieee80211_tim_parameters {
>  	u8 tim_period;
>  } __packed;
> 
> -//#else
>  struct ieee80211_wmm_ts_info {
>  	u8 ac_dir_tid;
>  	u8 ac_up_psb;
> @@ -1274,7 +1267,6 @@ enum {WMM_all_frame, WMM_two_frame, WMM_four_frame, WMM_six_frame};
>  #define IEEE80211_PS_MBCAST IEEE80211_DTIM_MBCAST
> 
>  /* added by David for QoS 2006/6/30 */
> -//#define WMM_Hang_8187
>  #ifdef WMM_Hang_8187
>  #undef WMM_Hang_8187
>  #endif

Are you sure you should remove this commented out line?

thanks,

greg k-h