On 07/02/2015 09:40 PM, Krzysztof Opasiak wrote:
On 07/02/2015 09:29 PM, Michal Nazarewicz wrote:
On Thu, Jul 02 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.
Also change the nluns to max_luns which is better name for value
stored in that place as we no longer need to store size of luns
array.
Reported-by: David Fisher <david.fisher1@xxxxxxxxxxxx>
Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
---
drivers/usb/gadget/function/f_mass_storage.c | 125
++++++++++----------------
drivers/usb/gadget/function/f_mass_storage.h | 4 -
drivers/usb/gadget/legacy/acm_ms.c | 6 --
drivers/usb/gadget/legacy/mass_storage.c | 6 --
drivers/usb/gadget/legacy/multi.c | 6 --
5 files changed, 48 insertions(+), 99 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index ad0f69b..befe251 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -279,9 +279,9 @@ struct fsg_common {
int cmnd_size;
u8 cmnd[MAX_COMMAND_SIZE];
- unsigned int nluns;
+ int max_lun;
To be honest, I don’t like this change. It is more idiomatic to store
number of elements in an array rather than index of the last element.
Sooner or later someone will do for (i = 0; i < common->max_lun; ++i)
out of habit.
The reason why I did this was allowing not contiguous LUNs ids. As you
suggested in earlier discussion [1] we should allow user space shoot
themselves in the foot and tis is one of the consequence. What we should
really return in GET_MAX_LUN is last valid LUN id in our array, not
number of LUNs. Windows is paying attention to this value and for
examnple for luns: 0 5 6 it makes a different if you reply with nluns -1
or max_lun. In first option windows will see only one LUN while in
second all three of them.
As we increment max_lun only when adding a LUN currently it is not
possible to get out of bounds.
I'm looking on this code today an I see another option. We could simply
remove nluns from opts and always iterate:
for (i = 0; i < ARRAY_SIZE(luns); ++i)
if (luns[i]) {
/* Do something useful */
}
and add some simple function:
int _fsg_common_get_max_lun(luns)
{
int i;
for (i = ARRAY_SIZE(luns) - 1; i >= 0 && !luns[i]; --i)
return i;
}
with such approach we doesn't need to store number of elements in arrays
or max index. Instead of this we just do something only for elements
which has been allocated and determine max lun dynamically when handling
GET_MAX_LUN request. What do you thing about this Michal? Maybe you have
another idea how to solve this?
Cheers,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
