* staging: rtl8723bs: bug or pointless if else ?
@ 2018-06-28 7:43 Michael Straube
2018-06-28 8:22 ` Hans de Goede
2018-06-28 10:24 ` Greg Kroah-Hartman
0 siblings, 2 replies; 7+ messages in thread
From: Michael Straube @ 2018-06-28 7:43 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: hadess, hdegoede, Larry.Finger, devel, linux-kernel
Hi,
I stumbled upon the following if else construct in
drivers/staging/rtl8723bs/os_dep/sdio_intf.c:618
if (pwrpriv->bInternalAutoSuspend)
{
ret = rtw_resume_process(padapter);
}
else
{
if (pwrpriv->wowlan_mode || pwrpriv->wowlan_ap_mode)
{
ret = rtw_resume_process(padapter);
}
else
{
ret = rtw_resume_process(padapter);
}
}
It does not matter if the conditions are true or not,
ret is always set to:
ret = rtw_resume_process(padapter)
Is this a bug or is the if else construct just pointless?
Regards,
Michael
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: staging: rtl8723bs: bug or pointless if else ?
2018-06-28 7:43 staging: rtl8723bs: bug or pointless if else ? Michael Straube
@ 2018-06-28 8:22 ` Hans de Goede
2018-06-28 9:34 ` Bastien Nocera
2018-06-28 10:24 ` Greg Kroah-Hartman
1 sibling, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2018-06-28 8:22 UTC (permalink / raw)
To: Michael Straube, Greg Kroah-Hartman
Cc: hadess, Larry.Finger, devel, linux-kernel
Hi,
On 28-06-18 09:43, Michael Straube wrote:
> Hi,
>
> I stumbled upon the following if else construct in
> drivers/staging/rtl8723bs/os_dep/sdio_intf.c:618
>
> if (pwrpriv->bInternalAutoSuspend)
> {
> ret = rtw_resume_process(padapter);
> }
> else
> {
> if (pwrpriv->wowlan_mode || pwrpriv->wowlan_ap_mode)
> {
> ret = rtw_resume_process(padapter);
> }
> else
> {
> ret = rtw_resume_process(padapter);
> }
> }
>
> It does not matter if the conditions are true or not,
> ret is always set to:
>
> ret = rtw_resume_process(padapter)
>
> Is this a bug or is the if else construct just pointless?
It probably is just pointless, my guess would be that once
upon a time there was a difference in the paths and at some
point that difference went away.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: staging: rtl8723bs: bug or pointless if else ?
2018-06-28 8:22 ` Hans de Goede
@ 2018-06-28 9:34 ` Bastien Nocera
2018-06-28 15:00 ` Michael Straube
2018-06-28 20:36 ` Joe Perches
0 siblings, 2 replies; 7+ messages in thread
From: Bastien Nocera @ 2018-06-28 9:34 UTC (permalink / raw)
To: Hans de Goede, Michael Straube, Greg Kroah-Hartman
Cc: Larry.Finger, devel, linux-kernel
On Thu, 2018-06-28 at 10:22 +0200, Hans de Goede wrote:
> Hi,
>
> On 28-06-18 09:43, Michael Straube wrote:
> > Hi,
> >
> > I stumbled upon the following if else construct in
> > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:618
> >
> > if (pwrpriv->bInternalAutoSuspend)
> > {
> > ret = rtw_resume_process(padapter);
> > }
> > else
> > {
> > if (pwrpriv->wowlan_mode || pwrpriv-
> > >wowlan_ap_mode)
> > {
> > ret = rtw_resume_process(padapter);
> > }
> > else
> > {
> > ret = rtw_resume_process(padapter);
> > }
> > }
> >
> > It does not matter if the conditions are true or not,
> > ret is always set to:
> >
> > ret = rtw_resume_process(padapter)
> >
> > Is this a bug or is the if else construct just pointless?
>
> It probably is just pointless, my guess would be that once
> upon a time there was a difference in the paths and at some
> point that difference went away.
Quite:
https://github.com/hadess/rtl8723bs/blob/7d36e26f78bbc709844c12ad0c62e3e8503fdbc5/os_dep/linux/sdio_intf.c#L1757
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: staging: rtl8723bs: bug or pointless if else ?
2018-06-28 9:34 ` Bastien Nocera
@ 2018-06-28 15:00 ` Michael Straube
2018-06-28 15:01 ` Hans de Goede
2018-06-28 20:36 ` Joe Perches
1 sibling, 1 reply; 7+ messages in thread
From: Michael Straube @ 2018-06-28 15:00 UTC (permalink / raw)
To: Bastien Nocera, Hans de Goede, Greg Kroah-Hartman
Cc: Larry.Finger, devel, linux-kernel
On 06/28/18 11:34, Bastien Nocera wrote:
> On Thu, 2018-06-28 at 10:22 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 28-06-18 09:43, Michael Straube wrote:
>>> Hi,
>>>
>>> I stumbled upon the following if else construct in
>>> drivers/staging/rtl8723bs/os_dep/sdio_intf.c:618
>>>
>>> if (pwrpriv->bInternalAutoSuspend)
>>> {
>>> ret = rtw_resume_process(padapter);
>>> }
>>> else
>>> {
>>> if (pwrpriv->wowlan_mode || pwrpriv-
>>>> wowlan_ap_mode)
>>> {
>>> ret = rtw_resume_process(padapter);
>>> }
>>> else
>>> {
>>> ret = rtw_resume_process(padapter);
>>> }
>>> }
>>>
>>> It does not matter if the conditions are true or not,
>>> ret is always set to:
>>>
>>> ret = rtw_resume_process(padapter)
>>>
>>> Is this a bug or is the if else construct just pointless?
>>
>> It probably is just pointless, my guess would be that once
>> upon a time there was a difference in the paths and at some
>> point that difference went away.
>
> Quite:
> https://github.com/hadess/rtl8723bs/blob/7d36e26f78bbc709844c12ad0c62e3e8503fdbc5/os_dep/linux/sdio_intf.c#L1757
>
So it can be safly removed?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: staging: rtl8723bs: bug or pointless if else ?
2018-06-28 15:00 ` Michael Straube
@ 2018-06-28 15:01 ` Hans de Goede
0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-06-28 15:01 UTC (permalink / raw)
To: Michael Straube, Bastien Nocera, Greg Kroah-Hartman
Cc: Larry.Finger, devel, linux-kernel
Hi,
On 28-06-18 17:00, Michael Straube wrote:
> On 06/28/18 11:34, Bastien Nocera wrote:
>> On Thu, 2018-06-28 at 10:22 +0200, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 28-06-18 09:43, Michael Straube wrote:
>>>> Hi,
>>>>
>>>> I stumbled upon the following if else construct in
>>>> drivers/staging/rtl8723bs/os_dep/sdio_intf.c:618
>>>>
>>>> if (pwrpriv->bInternalAutoSuspend)
>>>> {
>>>> ret = rtw_resume_process(padapter);
>>>> }
>>>> else
>>>> {
>>>> if (pwrpriv->wowlan_mode || pwrpriv-
>>>>> wowlan_ap_mode)
>>>> {
>>>> ret = rtw_resume_process(padapter);
>>>> }
>>>> else
>>>> {
>>>> ret = rtw_resume_process(padapter);
>>>> }
>>>> }
>>>>
>>>> It does not matter if the conditions are true or not,
>>>> ret is always set to:
>>>>
>>>> ret = rtw_resume_process(padapter)
>>>>
>>>> Is this a bug or is the if else construct just pointless?
>>>
>>> It probably is just pointless, my guess would be that once
>>> upon a time there was a difference in the paths and at some
>>> point that difference went away.
>>
>> Quite:
>> https://github.com/hadess/rtl8723bs/blob/7d36e26f78bbc709844c12ad0c62e3e8503fdbc5/os_dep/linux/sdio_intf.c#L1757
>>
>
> So it can be safly removed?
Simplified to a single:
ret = rtw_resume_process(padapter);
Yes that should be fine.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: staging: rtl8723bs: bug or pointless if else ?
2018-06-28 9:34 ` Bastien Nocera
2018-06-28 15:00 ` Michael Straube
@ 2018-06-28 20:36 ` Joe Perches
1 sibling, 0 replies; 7+ messages in thread
From: Joe Perches @ 2018-06-28 20:36 UTC (permalink / raw)
To: Bastien Nocera, Hans de Goede, Michael Straube,
Greg Kroah-Hartman
Cc: Larry.Finger, devel, linux-kernel
On Thu, 2018-06-28 at 11:34 +0200, Bastien Nocera wrote:
> On Thu, 2018-06-28 at 10:22 +0200, Hans de Goede wrote:
> > On 28-06-18 09:43, Michael Straube wrote:
> > > ret = rtw_resume_process(padapter)
> > > Is this a bug or is the if else construct just pointless?
> > It probably is just pointless, my guess would be that once
> > upon a time there was a difference in the paths and at some
> > point that difference went away.
>
> Quite:
> https://github.com/hadess/rtl8723bs/blob/7d36e26f78bbc709844c12ad0c62e3e8503fdbc5/os_dep/linux/sdio_intf.c#L1757
That #ifdef forest is/was quite unsightly.
No wonder the realtek code is unmaintainable.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: staging: rtl8723bs: bug or pointless if else ?
2018-06-28 7:43 staging: rtl8723bs: bug or pointless if else ? Michael Straube
2018-06-28 8:22 ` Hans de Goede
@ 2018-06-28 10:24 ` Greg Kroah-Hartman
1 sibling, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2018-06-28 10:24 UTC (permalink / raw)
To: Michael Straube; +Cc: hadess, hdegoede, Larry.Finger, devel, linux-kernel
On Thu, Jun 28, 2018 at 09:43:47AM +0200, Michael Straube wrote:
> Hi,
>
> I stumbled upon the following if else construct in
> drivers/staging/rtl8723bs/os_dep/sdio_intf.c:618
>
> if (pwrpriv->bInternalAutoSuspend)
> {
> ret = rtw_resume_process(padapter);
> }
> else
> {
> if (pwrpriv->wowlan_mode || pwrpriv->wowlan_ap_mode)
> {
> ret = rtw_resume_process(padapter);
> }
> else
> {
> ret = rtw_resume_process(padapter);
> }
> }
>
> It does not matter if the conditions are true or not,
> ret is always set to:
>
> ret = rtw_resume_process(padapter)
>
> Is this a bug or is the if else construct just pointless?
Looks pretty pointless...
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-06-28 20:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-28 7:43 staging: rtl8723bs: bug or pointless if else ? Michael Straube
2018-06-28 8:22 ` Hans de Goede
2018-06-28 9:34 ` Bastien Nocera
2018-06-28 15:00 ` Michael Straube
2018-06-28 15:01 ` Hans de Goede
2018-06-28 20:36 ` Joe Perches
2018-06-28 10:24 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.