* [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used @ 2009-05-26 15:19 Jaswinder Singh Rajput 2009-05-26 15:28 ` James Bottomley 2009-05-26 21:36 ` Mike Christie 0 siblings, 2 replies; 6+ messages in thread From: Jaswinder Singh Rajput @ 2009-05-26 15:19 UTC (permalink / raw) To: James Bottomley, linux-scsi, Ingo Molnar, x86 maintainers, LKML I am watching this problem from long time in -tip. Subject: [PATCH] scsi: scsi_transport_iscsi.c initialize the variable before it get used In some cases, err will be used uninitialized. Also fixed compilation warning : CC drivers/scsi/scsi_transport_iscsi.o drivers/scsi/scsi_transport_iscsi.c: In function ‘iscsi_add_session’: drivers/scsi/scsi_transport_iscsi.c:678: warning: ‘err’ may be used uninitialized in this function Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com> --- drivers/scsi/scsi_transport_iscsi.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 0947954..e78cde2 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -675,7 +675,7 @@ int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id) struct iscsi_cls_host *ihost; unsigned long flags; unsigned int id = target_id; - int err; + int err = 0; ihost = shost->shost_data; session->sid = atomic_add_return(1, &iscsi_session_nr); -- 1.6.0.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used 2009-05-26 15:19 [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used Jaswinder Singh Rajput @ 2009-05-26 15:28 ` James Bottomley 2009-05-26 16:00 ` Jaswinder Singh Rajput 2009-05-26 21:36 ` Mike Christie 1 sibling, 1 reply; 6+ messages in thread From: James Bottomley @ 2009-05-26 15:28 UTC (permalink / raw) To: Jaswinder Singh Rajput; +Cc: linux-scsi, Ingo Molnar, x86 maintainers, LKML On Tue, 2009-05-26 at 20:49 +0530, Jaswinder Singh Rajput wrote: > I am watching this problem from long time in -tip. > > Subject: [PATCH] scsi: scsi_transport_iscsi.c initialize the variable before it get used > > In some cases, err will be used uninitialized. What cases? A simple theoretical analysis shows that this variable is always set. > CC drivers/scsi/scsi_transport_iscsi.o > drivers/scsi/scsi_transport_iscsi.c: In function ‘iscsi_add_session’: > drivers/scsi/scsi_transport_iscsi.c:678: warning: ‘err’ may be used > uninitialized in this function My version of gcc (4.3.3) correctly sees that the variable cannot be uninitialised ... what version are you using? If it's a popular version, we can always do the uninitialised_var() thing, but if it's just a non-standard compiler, I'd rather not mess up the source code to please gcc. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used 2009-05-26 15:28 ` James Bottomley @ 2009-05-26 16:00 ` Jaswinder Singh Rajput 2009-05-26 16:06 ` James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Jaswinder Singh Rajput @ 2009-05-26 16:00 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, Ingo Molnar, x86 maintainers, LKML Hello James, On Tue, 2009-05-26 at 10:28 -0500, James Bottomley wrote: > On Tue, 2009-05-26 at 20:49 +0530, Jaswinder Singh Rajput wrote: > > I am watching this problem from long time in -tip. > > > > Subject: [PATCH] scsi: scsi_transport_iscsi.c initialize the variable before it get used > > > > In some cases, err will be used uninitialized. > > What cases? A simple theoretical analysis shows that this variable is > always set. > Ok lets takes your tree: http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=blob;f=drivers/scsi/scsi_transport_iscsi.c;hb=10eb0f013c63c71c82ede77945a5f390c10cfda6 672 int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id) 673 { 674 struct Scsi_Host *shost = iscsi_session_to_shost(session); 675 struct iscsi_cls_host *ihost; 676 unsigned long flags; 677 unsigned int id = target_id; 678 int err; 679 680 ihost = shost->shost_data; 681 session->sid = atomic_add_return(1, &iscsi_session_nr); 682 683 if (id == ISCSI_MAX_TARGET) { 684 for (id = 0; id < ISCSI_MAX_TARGET; id++) { 685 err = device_for_each_child(&shost->shost_gendev, &id, 686 iscsi_get_next_target_id); 687 if (!err) 688 break; 689 } 690 691 if (id == ISCSI_MAX_TARGET) { 692 iscsi_cls_session_printk(KERN_ERR, session, 693 "Too many iscsi targets. Max " 694 "number of targets is %d.\n", 695 ISCSI_MAX_TARGET - 1); 696 goto release_host; 697 } 698 } 699 session->target_id = id; 700 701 dev_set_name(&session->dev, "session%u", session->sid); 702 err = device_add(&session->dev); 703 if (err) { 704 iscsi_cls_session_printk(KERN_ERR, session, 705 "could not register session's dev\n"); 706 goto release_host; 707 } 708 transport_register_device(&session->dev); 709 710 spin_lock_irqsave(&sesslock, flags); 711 list_add(&session->sess_list, &sesslist); 712 spin_unlock_irqrestore(&sesslock, flags); 713 714 iscsi_session_event(session, ISCSI_KEVENT_CREATE_SESSION); 715 return 0; 716 717 release_host: 718 scsi_host_put(shost); 719 return err; 720 } You are right but compiler is confused and giving warning like this : 1. do not go inside : 684 for (id = 0; id < ISCSI_MAX_TARGET; id++) { 2. and then choose path : 696 goto release_host; then we return err uninitialized ;-) > > CC drivers/scsi/scsi_transport_iscsi.o > > drivers/scsi/scsi_transport_iscsi.c: In function ‘iscsi_add_session’: > > drivers/scsi/scsi_transport_iscsi.c:678: warning: ‘err’ may be used > > uninitialized in this function > > My version of gcc (4.3.3) correctly sees that the variable cannot be > uninitialised ... what version are you using? > > If it's a popular version, we can always do the uninitialised_var() > thing, but if it's just a non-standard compiler, I'd rather not mess up > the source code to please gcc. > gcc version 4.3.2 20081105 (Red Hat 4.3.2-7) (GCC) -- JSR -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used 2009-05-26 16:00 ` Jaswinder Singh Rajput @ 2009-05-26 16:06 ` James Bottomley 2009-05-26 16:21 ` Jaswinder Singh Rajput 0 siblings, 1 reply; 6+ messages in thread From: James Bottomley @ 2009-05-26 16:06 UTC (permalink / raw) To: Jaswinder Singh Rajput; +Cc: linux-scsi, Ingo Molnar, x86 maintainers, LKML On Tue, 2009-05-26 at 21:30 +0530, Jaswinder Singh Rajput wrote: > Hello James, > > On Tue, 2009-05-26 at 10:28 -0500, James Bottomley wrote: > > On Tue, 2009-05-26 at 20:49 +0530, Jaswinder Singh Rajput wrote: > > > I am watching this problem from long time in -tip. > > > > > > Subject: [PATCH] scsi: scsi_transport_iscsi.c initialize the variable before it get used > > > > > > In some cases, err will be used uninitialized. > > > > What cases? A simple theoretical analysis shows that this variable is > > always set. > > > > Ok lets takes your tree: > > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=blob;f=drivers/scsi/scsi_transport_iscsi.c;hb=10eb0f013c63c71c82ede77945a5f390c10cfda6 > > > 672 int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id) > 673 { > 674 struct Scsi_Host *shost = iscsi_session_to_shost(session); > 675 struct iscsi_cls_host *ihost; > 676 unsigned long flags; > 677 unsigned int id = target_id; > 678 int err; > 679 > 680 ihost = shost->shost_data; > 681 session->sid = atomic_add_return(1, &iscsi_session_nr); > 682 > 683 if (id == ISCSI_MAX_TARGET) { > 684 for (id = 0; id < ISCSI_MAX_TARGET; id++) { > 685 err = device_for_each_child(&shost->shost_gendev, &id, > 686 iscsi_get_next_target_id); > 687 if (!err) > 688 break; > 689 } > 690 > 691 if (id == ISCSI_MAX_TARGET) { > 692 iscsi_cls_session_printk(KERN_ERR, session, > 693 "Too many iscsi targets. Max " > 694 "number of targets is %d.\n", > 695 ISCSI_MAX_TARGET - 1); > 696 goto release_host; > 697 } > 698 } > 699 session->target_id = id; > 700 > 701 dev_set_name(&session->dev, "session%u", session->sid); > 702 err = device_add(&session->dev); > 703 if (err) { > 704 iscsi_cls_session_printk(KERN_ERR, session, > 705 "could not register session's dev\n"); > 706 goto release_host; > 707 } > 708 transport_register_device(&session->dev); > 709 > 710 spin_lock_irqsave(&sesslock, flags); > 711 list_add(&session->sess_list, &sesslist); > 712 spin_unlock_irqrestore(&sesslock, flags); > 713 > 714 iscsi_session_event(session, ISCSI_KEVENT_CREATE_SESSION); > 715 return 0; > 716 > 717 release_host: > 718 scsi_host_put(shost); > 719 return err; > 720 } > > You are right but compiler is confused and giving warning like this : > > 1. do not go inside : > 684 for (id = 0; id < ISCSI_MAX_TARGET; id++) { How? id is an unsigned int and ISCSI_MAX_TARGET is defined to be -1 in include/scsi/scsi_transport_iscsi.h (might be better if it were UINT_MAX, but -1 is just as good). James ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used 2009-05-26 16:06 ` James Bottomley @ 2009-05-26 16:21 ` Jaswinder Singh Rajput 0 siblings, 0 replies; 6+ messages in thread From: Jaswinder Singh Rajput @ 2009-05-26 16:21 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, Ingo Molnar, x86 maintainers, LKML On Tue, 2009-05-26 at 16:06 +0000, James Bottomley wrote: > On Tue, 2009-05-26 at 21:30 +0530, Jaswinder Singh Rajput wrote: > > Hello James, > > > > On Tue, 2009-05-26 at 10:28 -0500, James Bottomley wrote: > > > On Tue, 2009-05-26 at 20:49 +0530, Jaswinder Singh Rajput wrote: > > > > I am watching this problem from long time in -tip. > > > > > > > > Subject: [PATCH] scsi: scsi_transport_iscsi.c initialize the variable before it get used > > > > > > > > In some cases, err will be used uninitialized. > > > > > > What cases? A simple theoretical analysis shows that this variable is > > > always set. > > > > > > > Ok lets takes your tree: > > > > http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=blob;f=drivers/scsi/scsi_transport_iscsi.c;hb=10eb0f013c63c71c82ede77945a5f390c10cfda6 > > > > > > 672 int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id) > > 673 { > > 674 struct Scsi_Host *shost = iscsi_session_to_shost(session); > > 675 struct iscsi_cls_host *ihost; > > 676 unsigned long flags; > > 677 unsigned int id = target_id; > > 678 int err; > > 679 > > 680 ihost = shost->shost_data; > > 681 session->sid = atomic_add_return(1, &iscsi_session_nr); > > 682 > > 683 if (id == ISCSI_MAX_TARGET) { > > 684 for (id = 0; id < ISCSI_MAX_TARGET; id++) { > > 685 err = device_for_each_child(&shost->shost_gendev, &id, > > 686 iscsi_get_next_target_id); > > 687 if (!err) > > 688 break; > > 689 } > > 690 > > 691 if (id == ISCSI_MAX_TARGET) { > > 692 iscsi_cls_session_printk(KERN_ERR, session, > > 693 "Too many iscsi targets. Max " > > 694 "number of targets is %d.\n", > > 695 ISCSI_MAX_TARGET - 1); > > 696 goto release_host; > > 697 } > > 698 } > > 699 session->target_id = id; > > 700 > > 701 dev_set_name(&session->dev, "session%u", session->sid); > > 702 err = device_add(&session->dev); > > 703 if (err) { > > 704 iscsi_cls_session_printk(KERN_ERR, session, > > 705 "could not register session's dev\n"); > > 706 goto release_host; > > 707 } > > 708 transport_register_device(&session->dev); > > 709 > > 710 spin_lock_irqsave(&sesslock, flags); > > 711 list_add(&session->sess_list, &sesslist); > > 712 spin_unlock_irqrestore(&sesslock, flags); > > 713 > > 714 iscsi_session_event(session, ISCSI_KEVENT_CREATE_SESSION); > > 715 return 0; > > 716 > > 717 release_host: > > 718 scsi_host_put(shost); > > 719 return err; > > 720 } > > > > You are right but compiler is confused and giving warning like this : > > > > 1. do not go inside : > > 684 for (id = 0; id < ISCSI_MAX_TARGET; id++) { > > How? id is an unsigned int and ISCSI_MAX_TARGET is defined to be -1 in > include/scsi/scsi_transport_iscsi.h (might be better if it were > UINT_MAX, but -1 is just as good). > Yes, you are right, I was telling you how compiler was confused in 4.3.2 but I am glad to hear that this problem is fixed in 4.3.3 as you told me :-) So now its upto you. 4.3.2 is not soo old, So either initialize the variable or stay with stray warning in the kernel ;-) Thanks, -- JSR ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used 2009-05-26 15:19 [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used Jaswinder Singh Rajput 2009-05-26 15:28 ` James Bottomley @ 2009-05-26 21:36 ` Mike Christie 1 sibling, 0 replies; 6+ messages in thread From: Mike Christie @ 2009-05-26 21:36 UTC (permalink / raw) To: Jaswinder Singh Rajput Cc: James Bottomley, linux-scsi, Ingo Molnar, x86 maintainers, LKML [-- Attachment #1: Type: text/plain, Size: 916 bytes --] Jaswinder Singh Rajput wrote: > I am watching this problem from long time in -tip. > > Subject: [PATCH] scsi: scsi_transport_iscsi.c initialize the variable before it get used > > In some cases, err will be used uninitialized. > As you guys already discussed it will not get used uninitialized. > Also fixed compilation warning : > > CC drivers/scsi/scsi_transport_iscsi.o > drivers/scsi/scsi_transport_iscsi.c: In function ‘iscsi_add_session’: > drivers/scsi/scsi_transport_iscsi.c:678: warning: ‘err’ may be used uninitialized in this function > I did the attached patch to silence the compile warning. I goofed and forgot to send it before, because I had change my compiler and was not seeing the error message. I was planning on sending the attached with some other changes for the next feature window. I am going to fix up the patch to use UINT_MAX like James mentioned in the thread. [-- Attachment #2: 0001-iscsi-class-quiet-iscsi_add_session-compilation-war.patch --] [-- Type: text/x-patch, Size: 1221 bytes --] >From 32ffa5248bf11f4df76ebf3c4a023958a9131f9c Mon Sep 17 00:00:00 2001 From: Mike Christie <michaelc@cs.wisc.edu> Date: Mon, 18 May 2009 11:54:37 -0500 Subject: [PATCH] iscsi class: quiet iscsi_add_session compilation warning. If ISCSI_MAX_TARGET was zero then iscsi_add_session could return success when it had failed. This will currently not happen because ISCSI_MAX_TARGET is a macro and not a variable. But gcc likes to complain about it and people keep reporting it. This patch just initializes err to be -EINVAL so if we ever defined ISCSI_MAX_TARGET incorrectly we would fail gracefully. --- drivers/scsi/scsi_transport_iscsi.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 0a2ce7b..f508567 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -675,7 +675,7 @@ int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id) struct iscsi_cls_host *ihost; unsigned long flags; unsigned int id = target_id; - int err; + int err = -EINVAL; ihost = shost->shost_data; session->sid = atomic_add_return(1, &iscsi_session_nr); -- 1.6.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-05-26 21:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-26 15:19 [PATCH -tip][SCSI] scsi: scsi_transport_iscsi.c initialize the variable before it get used Jaswinder Singh Rajput 2009-05-26 15:28 ` James Bottomley 2009-05-26 16:00 ` Jaswinder Singh Rajput 2009-05-26 16:06 ` James Bottomley 2009-05-26 16:21 ` Jaswinder Singh Rajput 2009-05-26 21:36 ` Mike Christie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox