[PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Maxin John]

Hi,

Since comparing an unsigned int (common->lun)  greater than or equal
to zero is always true, I think it is safe to remove this check.
Please let me know your comments.

Thanks to Coverity for pointing this out.

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
---
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 6d8e533..cade79e 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1910,7 +1910,7 @@ static int check_command(struct fsg_common
*common, int cmnd_size,
                    common->lun, lun);

        /* Check the LUN */
-       if (common->lun >= 0 && common->lun < common->nluns) {
+       if (common->lun < common->nluns) {
                curlun = &common->luns[common->lun];
                common->curlun = curlun;
                if (common->cmnd[0] != REQUEST_SENSE) {

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Michal Nazarewicz]

On Wed, 04 May 2011 16:16:05 +0200, Maxin John <maxin.john@gmail.com>  
wrote:
> Since comparing an unsigned int (common->lun)  greater than or equal
> to zero is always true, I think it is safe to remove this check.
> Please let me know your comments.
>
> Thanks to Coverity for pointing this out.
>
> Signed-off-by: Maxin B. John <maxin.john@gmail.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

file_storage.c has the same check, could you remove it as well.

> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -1910,7 +1910,7 @@ static int check_command(struct fsg_common
> *common, int cmnd_size,
>                     common->lun, lun);
>
>         /* Check the LUN */
> -       if (common->lun >= 0 && common->lun < common->nluns) {
> +       if (common->lun < common->nluns) {
>                 curlun = &common->luns[common->lun];
>                 common->curlun = curlun;
>                 if (common->cmnd[0] != REQUEST_SENSE) {


-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Maxin John]

Hi Michal,

> Acked-by: Michal Nazarewicz <mina86@mina86.com>

Thank you very much for reviewing the patch.

> file_storage.c has the same check, could you remove it as well.

Please find the patch for "file_storage.c" below. Should I merge these
two patches and re-submit as a single one?

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
---
diff --git a/drivers/usb/gadget/file_storage.c
b/drivers/usb/gadget/file_storage.c
index a6eacb5..a9c5177 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -2314,7 +2314,7 @@ static int check_command(struct fsg_dev *fsg,
int cmnd_size,
                fsg->lun = lun;         // Use LUN from the command

        /* Check the LUN */
-       if (fsg->lun >= 0 && fsg->lun < fsg->nluns) {
+       if (fsg->lun < fsg->nluns) {
                fsg->curlun = curlun = &fsg->luns[fsg->lun];
                if (fsg->cmnd[0] != REQUEST_SENSE) {
                        curlun->sense_data = SS_NO_SENSE;

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Michal Nazarewicz]

On Wed, 04 May 2011 17:04:03 +0200, Maxin John <maxin.john@gmail.com>  
wrote:

> Hi Michal,
>
>> Acked-by: Michal Nazarewicz <mina86@mina86.com>
>
> Thank you very much for reviewing the patch.
>
>> file_storage.c has the same check, could you remove it as well.
>
> Please find the patch for "file_storage.c" below. Should I merge these
> two patches and re-submit as a single one?

I would merge them together.

> Signed-off-by: Maxin B. John <maxin.john@gmail.com>
> ---
> diff --git a/drivers/usb/gadget/file_storage.c
> b/drivers/usb/gadget/file_storage.c
> index a6eacb5..a9c5177 100644
> --- a/drivers/usb/gadget/file_storage.c
> +++ b/drivers/usb/gadget/file_storage.c
> @@ -2314,7 +2314,7 @@ static int check_command(struct fsg_dev *fsg,
> int cmnd_size,
>                 fsg->lun = lun;         // Use LUN from the command
>
>         /* Check the LUN */
> -       if (fsg->lun >= 0 && fsg->lun < fsg->nluns) {
> +       if (fsg->lun < fsg->nluns) {
>                 fsg->curlun = curlun = &fsg->luns[fsg->lun];
>                 if (fsg->cmnd[0] != REQUEST_SENSE) {
>                         curlun->sense_data = SS_NO_SENSE;


-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Alan Stern]

On Wed, 4 May 2011, Maxin John wrote:

> Hi Michal,
> 
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> 
> Thank you very much for reviewing the patch.
> 
> > file_storage.c has the same check, could you remove it as well.
> 
> Please find the patch for "file_storage.c" below. Should I merge these
> two patches and re-submit as a single one?
> 
> Signed-off-by: Maxin B. John <maxin.john@gmail.com>

Acked-by: Alan Stern <stern@rowland.harvard.edu>

> ---
> diff --git a/drivers/usb/gadget/file_storage.c
> b/drivers/usb/gadget/file_storage.c
> index a6eacb5..a9c5177 100644
> --- a/drivers/usb/gadget/file_storage.c
> +++ b/drivers/usb/gadget/file_storage.c
> @@ -2314,7 +2314,7 @@ static int check_command(struct fsg_dev *fsg,
> int cmnd_size,
>                 fsg->lun = lun;         // Use LUN from the command
> 
>         /* Check the LUN */
> -       if (fsg->lun >= 0 && fsg->lun < fsg->nluns) {
> +       if (fsg->lun < fsg->nluns) {
>                 fsg->curlun = curlun = &fsg->luns[fsg->lun];
>                 if (fsg->cmnd[0] != REQUEST_SENSE) {
>                         curlun->sense_data = SS_NO_SENSE;

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Greg KH]

On Wed, May 04, 2011 at 04:04:03PM +0100, Maxin John wrote:
> Hi Michal,
> 
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> 
> Thank you very much for reviewing the patch.
> 
> > file_storage.c has the same check, could you remove it as well.
> 
> Please find the patch for "file_storage.c" below. Should I merge these
> two patches and re-submit as a single one?

Yes please.

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Maxin John]

Hi Greg,

>> Please find the patch for "file_storage.c" below. Should I merge these
>> two patches and re-submit as a single one?
>
> Yes please.

As per your suggestion, please find the merged patch below:

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
diff --git a/drivers/usb/gadget/f_mass_storage.c
b/drivers/usb/gadget/f_mass_storage.c
index 6d8e533..cade79e 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -1910,7 +1910,7 @@ static int check_command(struct fsg_common
*common, int cmnd_size,
                    common->lun, lun);

        /* Check the LUN */
-       if (common->lun >= 0 && common->lun < common->nluns) {
+       if (common->lun < common->nluns) {
                curlun = &common->luns[common->lun];
                common->curlun = curlun;
                if (common->cmnd[0] != REQUEST_SENSE) {
diff --git a/drivers/usb/gadget/file_storage.c
b/drivers/usb/gadget/file_storage.c
index a6eacb5..a9c5177 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -2314,7 +2314,7 @@ static int check_command(struct fsg_dev *fsg,
int cmnd_size,
                fsg->lun = lun;         // Use LUN from the command

        /* Check the LUN */
-       if (fsg->lun >= 0 && fsg->lun < fsg->nluns) {
+       if (fsg->lun < fsg->nluns) {
                fsg->curlun = curlun = &fsg->luns[fsg->lun];
                if (fsg->cmnd[0] != REQUEST_SENSE) {
                        curlun->sense_data = SS_NO_SENSE;

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Greg KH]

On Sat, May 07, 2011 at 12:27:58PM +0300, Maxin John wrote:
> Hi Greg,
> 
> >> Please find the patch for "file_storage.c" below. Should I merge these
> >> two patches and re-submit as a single one?
> >
> > Yes please.
> 
> As per your suggestion, please find the merged patch below:
> 
> Signed-off-by: Maxin B. John <maxin.john@gmail.com>
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Ok, but you forgot a changelog entry and a good subject line.

Care to redo this in a format that I don't have to edit it in order to
be able to apply it properly?

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_mass_storage: Remove the LUN check which is always true

[Maxin John]

Hi Greg,

On Sat, May 7, 2011 at 6:43 PM, Greg KH <greg@kroah.com> wrote:
> On Sat, May 07, 2011 at 12:27:58PM +0300, Maxin John wrote:
>> Hi Greg,
>
> Ok, but you forgot a changelog entry and a good subject line.

Oh.. my mistake. I forgot to update it.. Sorry.

> Care to redo this in a format that I don't have to edit it in order to
> be able to apply it properly?

I think, I should blame Gmail for this.  I have switched to "mutt" as
it seems to work fine for me now. I will re-send the merged patch based
on your suggestions.

Thanks,
Maxin