Re: [PATCH v3 4/5] usb: gadget: mass_storage: Use static array for luns

[Krzysztof Opasiak <k.opasiak@samsung.com>]

On 07/20/2015 03:17 PM, Michal Nazarewicz wrote:
>> On 07/08/2015 04:00 PM, Michal Nazarewicz wrote:
>>> On Tue, Jul 07 2015, Krzysztof Opasiak wrote:
>>>> This patch replace dynamicly allocated luns array with static one.
>>>> This simplifies the code of mass storage function and modules.
>>>>
>>>> It also fix issue with reporting wrong number of LUNs in GET_MAX_LUN
>>>> request. According to MS spec we should return the max index of valid
>>>> LUN, not the number of luns - 1.
>>>
>>> This is no longer true, is it?  Why my fix this bug has been solved, no?
>>> As such, this should not go to stable.  Or am I missing something?
>
> On Mon, Jul 20 2015, Krzysztof Opasiak wrote:
>> Unfortunately it's still true. Your fix solved this bug partially. Now
>> we report nluns - 1 instead of FSG_LUN_MAX but in case of not contiguous
>> luns it is not enough.
>>
>> Let's consider mass storage function with luns 0 1 3 5. nluns == 4 so in
>> GET_MAX_LUN we will return 3 (nluns - 1).
>
> I don’t believe that’s accurate.  See loop in my patch:
>
> +       if (!opts->refcnt) {
> +               for (nluns = i = 0; i < FSG_MAX_LUNS; ++i)
> +                       if (common->luns[i])
> +                               nluns = i + 1;
> +               if (!nluns)
> +                       pr_warn("No LUNS defined, continuing anyway\n");
> +               else
> +                       common->nluns = nluns;
> +               pr_info("Number of LUNs=%u\n", common->nluns);
> +       }
>
> It iterates over all luns and if lun is non-NULL it sets nluns to index
> of that lun plus one.  So if luns[0], lun[1], lun[2], lun[3] and lun[5]
> are set, common->nluns = 6.
>

Yeah I see it now. What I wrote was a problem before your patch but now 
it seams to be ok, just nluns name is very misleading. Summing up, just 
like you wrote this patch should not go to stable.

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics