On 4-4-2017 20:53, Hans de Goede wrote:
> Hi,
>
> On 04/04/2017 08:31 PM, Larry Finger wrote:
>> On 03/29/2017 12:47 PM, Hans de Goede wrote:
>>> The rtl8723bs is found on quite a few systems used by Linux users,
>>> such as on Atom systems (Intel Computestick and various other
>>> Atom based devices) and on many (budget) ARM boards such as
>>> the CHIP.
>>>
>>> The plan moving forward with this is for the new clean,
>>> written from scratch, rtl8xxxu driver to eventually gain
>>> support for sdio devices. But there is no clear timeline
>>> for that, so lets add this driver included in staging for now.
>>
>> Hans,
>>
>> I started looking at the Smatch errors. This one may be the result of
>> a serious problem:
>>
>> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c
>> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info()
>> error: we previously assumed 'phead' could be null (see line 453)
>> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info()
>> warn: variable dereferenced before check 'phead' (see line 454)
>>
>> A snippet of the code in question is as follows:
>>
>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
>> phead = get_list_head(queue);
>> plist = phead ? get_next(phead) : NULL;
>> plist = get_next(phead);
>> if ((!phead) || (!plist)) {
>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
>> return 0;
>> }
>>
>> This code comes directly from the hadess repo, but I am suspicious of
>> the double call to get_next(phead). I cannot imagine any valid reason
>> to skip every other entry on that list.
>
> If you look closer and simplify the first getnext line what is written is:
>
> plist = get_next(phead);
> plist = get_next(phead);
>
> Which indeed looks like it could use improvement, but I don't
> think it is seriously broken.
So get_list_head(queue) can never return NULL? Otherwise I don't know
how close I need to get for a simplified look ;-)
Gr. AvS
> Regards,
>
> Hans
