Re: [PATCH] staging: rtl8192u: Merge almost duplicate code
On Sun, May 17, 2020 at 09:25:05PM +0100, Pascal Terjan wrote: > On Sun, 17 May 2020 at 17:58, Pascal Terjan wrote: > > > > This causes a change in behaviour: > > - stats also get updated when reordering, this seems like it should be > > the case but those lines were commented out. > > - sub_skb NULL check now happens early in both cases, previously it > > happened only after dereferencing it 12 times, so it may not actually > > be needed. > > > > Hi, > I actually noticed the same duplicated code (and same late NULL check) > in drivers/staging/rtl8192e/rtllib_rx.c > drivers/staging/rtl8712/rtl8712_recv.c has only one copy of the code > but with the late NULL check > drivers/staging/rtl8188eu/core/rtw_recv.c has only one copy of the > code and doesn't do any NULL check > > Now I wonder how to proceed. The code is not great so it would not > feel right to make it reusable. > Should I continue improving it on this driver only first (maybe trying > to reuse ieee80211_data_to_8023_exthdr from net/wireless/util.c for > example)? It looks like the NULL check could be removed, but it's also fine to keep it so long as it's not after a NULL dereference. Do whatever you have the energy to do... It would be nice if people who fix bugs in these Realtek drivers would check the other drivers as well but for cleanups basically everyone just works on one driver only. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: Merge almost duplicate code
Hi Pascal, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on staging/staging-testing] [also build test WARNING on v5.7-rc5 next-20200515] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Pascal-Terjan/staging-rtl8192u-Merge-almost-duplicate-code/20200518-005912 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git cef077e6aa4c7dbe2f23e1201cf705f9540ec467 config: i386-allyesconfig (attached as .config) reproduce: # apt-get install sparse # sparse version: v0.6.1-193-gb8fad4bc-dirty # save the attached .config to linux build tree make C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot sparse warnings: (new ones prefixed by >>) /usr/lib/gcc/x86_64-linux-gnu/7/include/stddef.h:417:9: sparse: sparse: preprocessor token offsetof redefined include/linux/stddef.h:17:9: sparse: this was the original definition >> drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c:523:6: sparse: sparse: >> symbol 'indicate_packets' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
