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 <[email protected]>
---
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) {
On Wed, 04 May 2011 16:16:05 +0200, Maxin John <[email protected]>
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 <[email protected]>
Acked-by: Michal Nazarewicz <[email protected]>
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: [email protected]>-----ooO--(_)--Ooo--
Hi Michal,
> Acked-by: Michal Nazarewicz <[email protected]>
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 <[email protected]>
---
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;
On Wed, 04 May 2011 17:04:03 +0200, Maxin John <[email protected]>
wrote:
> Hi Michal,
>
>> Acked-by: Michal Nazarewicz <[email protected]>
>
> 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 <[email protected]>
> ---
> 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: [email protected]>-----ooO--(_)--Ooo--
On Wed, 4 May 2011, Maxin John wrote:
> Hi Michal,
>
> > Acked-by: Michal Nazarewicz <[email protected]>
>
> 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 <[email protected]>
Acked-by: Alan Stern <[email protected]>
> ---
> 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;
On Wed, May 04, 2011 at 04:04:03PM +0100, Maxin John wrote:
> Hi Michal,
>
> > Acked-by: Michal Nazarewicz <[email protected]>
>
> 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
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 <[email protected]>
Acked-by: Michal Nazarewicz <[email protected]>
Acked-by: Alan Stern <[email protected]>
---
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;
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 <[email protected]>
> Acked-by: Michal Nazarewicz <[email protected]>
> Acked-by: Alan Stern <[email protected]>
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
Hi Greg,
On Sat, May 7, 2011 at 6:43 PM, Greg KH <[email protected]> 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