Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 18 May 2020 16:25:23 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Pascal Terjan <pterjan@google.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8192u: Merge almost duplicate code
Message-ID: <20200518132523.GK2078@kadam>
References: <20200517165819.136715-1-pterjan@google.com>
 <CAANdO=Li7FUbVQk6m+CksZBv1zy-F+-1tN9oYZ4niYJ0utRfXA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAANdO=Li7FUbVQk6m+CksZBv1zy-F+-1tN9oYZ4niYJ0utRfXA@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, May 17, 2020 at 09:25:05PM +0100, Pascal Terjan wrote:
> On Sun, 17 May 2020 at 17:58, Pascal Terjan <pterjan@google.com> 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