>-----Original Message----- >From: Kevin Wolf [mailto:[email protected]] >Sent: Thursday, February 27, 2020 6:31 PM >To: Chenqun (kuhn) <[email protected]> >Cc: [email protected]; [email protected]; >[email protected]; Zhanghailiang <[email protected]>; >Euler Robot <[email protected]>; Ronnie Sahlberg ><[email protected]>; Paolo Bonzini <[email protected]>; Peter >Lieven <[email protected]>; Max Reitz <[email protected]> >Subject: Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement in >iscsi_open() > >Am 27.02.2020 um 02:49 hat Chenqun (kuhn) geschrieben: >> >-----Original Message----- >> >From: Kevin Wolf [mailto:[email protected]] >> >Sent: Wednesday, February 26, 2020 5:55 PM >> >To: Chenqun (kuhn) <[email protected]> >> >Cc: [email protected]; [email protected]; >> >[email protected]; Zhanghailiang >> ><[email protected]>; Euler Robot >> ><[email protected]>; Ronnie Sahlberg ><[email protected]>; >> >Paolo Bonzini <[email protected]>; Peter Lieven <[email protected]>; Max >> >Reitz <[email protected]> >> >Subject: Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement >> >in >> >iscsi_open() >> > >> >Am 26.02.2020 um 09:46 hat [email protected] geschrieben: >> >> From: Chen Qun <[email protected]> >> >> >> >> Clang static code analyzer show warning: >> >> block/iscsi.c:1920:9: warning: Value stored to 'flags' is never read >> >> flags &= ~BDRV_O_RDWR; >> >> ^ ~~~~~~~~~~~~ >> >> >> >> Reported-by: Euler Robot <[email protected]> >> >> Signed-off-by: Chen Qun <[email protected]> >> > >> >Hmm, I'm not so sure about this one because if we remove the line, >> >flags will be inconsistent with bs->open_flags. It feels like setting >> >a trap for anyone who wants to add code using flags in the future. >> Hi Kevin, >> I find it exists since 8f3bf50d34037266. : ) > >Yes, it has existed from the start with auto-read-only. > >> It's not a big deal, just upset clang static code analyzer. >> As you said, it could be a trap for the future. > >What's interesting is that we do have one user of the flags later in the >function, >but it uses bs->open_flags instead: > > ret = iscsi_allocmap_init(iscsilun, bs->open_flags); > Good point, I think this is exactly where the 'flags' are needed now. It should be right to keep it for the future, too.
Later version, I will modify it. Thanks. > >Maybe this should be using flags? (The value of the bits we're interested in is >the same, but when flags is passed as a parameter, I would expect it to be >used.)
