* drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] [not found] <202203021333.mMJpWPzx-lkp@intel.com> @ 2022-03-02 9:47 ` kernel test robot 2022-03-02 10:32 ` Marc Kleine-Budde 0 siblings, 1 reply; 6+ messages in thread From: kernel test robot @ 2022-03-02 9:47 UTC (permalink / raw) To: Vincent Mailhol Cc: kbuild-all, Linux Kernel Mailing List, Marc Kleine-Budde, Arunachalam Santhanam tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 575115360652e9920cc56a028a286ebe9bf82694 commit: c664e2137a27680922d8aeb64fb10313416b254f can: etas_es58x: add support for the ETAS ES58X_FD CAN USB interfaces date: 11 months ago compiler: powerpc64-linux-gcc (GCC) 11.2.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <yujie.liu@intel.com> cppcheck possible warnings: (new ones prefixed by >>, may not be real problems) In file included from drivers/net/can/usb/etas_es58x/es58x_fd.c: >> drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] ret = es58x_check_msg_len(es58x_dev->dev, *rx_event_msg, msg_len); ^ vim +174 drivers/net/can/usb/etas_es58x/es58x_fd.c c664e2137a2768 Vincent Mailhol 2021-04-10 165 c664e2137a2768 Vincent Mailhol 2021-04-10 166 static int es58x_fd_rx_event_msg(struct net_device *netdev, c664e2137a2768 Vincent Mailhol 2021-04-10 167 const struct es58x_fd_urb_cmd *es58x_fd_urb_cmd) c664e2137a2768 Vincent Mailhol 2021-04-10 168 { c664e2137a2768 Vincent Mailhol 2021-04-10 169 struct es58x_device *es58x_dev = es58x_priv(netdev)->es58x_dev; c664e2137a2768 Vincent Mailhol 2021-04-10 170 u16 msg_len = get_unaligned_le16(&es58x_fd_urb_cmd->msg_len); c664e2137a2768 Vincent Mailhol 2021-04-10 @171 const struct es58x_fd_rx_event_msg *rx_event_msg; c664e2137a2768 Vincent Mailhol 2021-04-10 172 int ret; c664e2137a2768 Vincent Mailhol 2021-04-10 173 c664e2137a2768 Vincent Mailhol 2021-04-10 @174 ret = es58x_check_msg_len(es58x_dev->dev, *rx_event_msg, msg_len); c664e2137a2768 Vincent Mailhol 2021-04-10 175 if (ret) c664e2137a2768 Vincent Mailhol 2021-04-10 176 return ret; c664e2137a2768 Vincent Mailhol 2021-04-10 177 c664e2137a2768 Vincent Mailhol 2021-04-10 178 rx_event_msg = &es58x_fd_urb_cmd->rx_event_msg; c664e2137a2768 Vincent Mailhol 2021-04-10 179 c664e2137a2768 Vincent Mailhol 2021-04-10 180 return es58x_rx_err_msg(netdev, rx_event_msg->error_code, c664e2137a2768 Vincent Mailhol 2021-04-10 181 rx_event_msg->event_code, c664e2137a2768 Vincent Mailhol 2021-04-10 182 get_unaligned_le64(&rx_event_msg->timestamp)); c664e2137a2768 Vincent Mailhol 2021-04-10 183 } c664e2137a2768 Vincent Mailhol 2021-04-10 184 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] 2022-03-02 9:47 ` drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] kernel test robot @ 2022-03-02 10:32 ` Marc Kleine-Budde 2022-03-02 12:49 ` Vincent MAILHOL 0 siblings, 1 reply; 6+ messages in thread From: Marc Kleine-Budde @ 2022-03-02 10:32 UTC (permalink / raw) To: kernel test robot Cc: Vincent Mailhol, kbuild-all, Linux Kernel Mailing List, Arunachalam Santhanam [-- Attachment #1: Type: text/plain, Size: 3372 bytes --] On 02.03.2022 17:47:08, kernel test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > head: 575115360652e9920cc56a028a286ebe9bf82694 > commit: c664e2137a27680922d8aeb64fb10313416b254f can: etas_es58x: add support for the ETAS ES58X_FD CAN USB interfaces > date: 11 months ago > compiler: powerpc64-linux-gcc (GCC) 11.2.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <yujie.liu@intel.com> > > > cppcheck possible warnings: (new ones prefixed by >>, may not be real problems) > > In file included from drivers/net/can/usb/etas_es58x/es58x_fd.c: > >> drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] > ret = es58x_check_msg_len(es58x_dev->dev, *rx_event_msg, msg_len); > ^ > > vim +174 drivers/net/can/usb/etas_es58x/es58x_fd.c > > c664e2137a2768 Vincent Mailhol 2021-04-10 165 > c664e2137a2768 Vincent Mailhol 2021-04-10 166 static int es58x_fd_rx_event_msg(struct net_device *netdev, > c664e2137a2768 Vincent Mailhol 2021-04-10 167 const struct es58x_fd_urb_cmd *es58x_fd_urb_cmd) > c664e2137a2768 Vincent Mailhol 2021-04-10 168 { > c664e2137a2768 Vincent Mailhol 2021-04-10 169 struct es58x_device *es58x_dev = es58x_priv(netdev)->es58x_dev; > c664e2137a2768 Vincent Mailhol 2021-04-10 170 u16 msg_len = get_unaligned_le16(&es58x_fd_urb_cmd->msg_len); > c664e2137a2768 Vincent Mailhol 2021-04-10 @171 const struct es58x_fd_rx_event_msg *rx_event_msg; > c664e2137a2768 Vincent Mailhol 2021-04-10 172 int ret; > c664e2137a2768 Vincent Mailhol 2021-04-10 173 > c664e2137a2768 Vincent Mailhol 2021-04-10 @174 ret = es58x_check_msg_len(es58x_dev->dev, *rx_event_msg, msg_len); > c664e2137a2768 Vincent Mailhol 2021-04-10 175 if (ret) > c664e2137a2768 Vincent Mailhol 2021-04-10 176 return ret; > c664e2137a2768 Vincent Mailhol 2021-04-10 177 > c664e2137a2768 Vincent Mailhol 2021-04-10 178 rx_event_msg = &es58x_fd_urb_cmd->rx_event_msg; > c664e2137a2768 Vincent Mailhol 2021-04-10 179 > c664e2137a2768 Vincent Mailhol 2021-04-10 180 return es58x_rx_err_msg(netdev, rx_event_msg->error_code, > c664e2137a2768 Vincent Mailhol 2021-04-10 181 rx_event_msg->event_code, > c664e2137a2768 Vincent Mailhol 2021-04-10 182 get_unaligned_le64(&rx_event_msg->timestamp)); > c664e2137a2768 Vincent Mailhol 2021-04-10 183 } > c664e2137a2768 Vincent Mailhol 2021-04-10 184 Thanks for the report. This looks like a false positive to me, as es58x_check_msg_len() is not a function, but a macro: | #define es58x_check_msg_len(dev, msg, actual_len) \ | __es58x_check_msg_len(dev, __stringify(msg), \ | actual_len, sizeof(msg)) __es58x_check_msg_len() don't use "rx_event_msg" directly, but only a string representation of it and a "sizeof()". I think it's possible to assign rx_event_msg before the es58x_check_msg_len(). I think (hope?) the compiler will not optimize anything away. :) regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] 2022-03-02 10:32 ` Marc Kleine-Budde @ 2022-03-02 12:49 ` Vincent MAILHOL 2022-03-02 13:04 ` Marc Kleine-Budde 0 siblings, 1 reply; 6+ messages in thread From: Vincent MAILHOL @ 2022-03-02 12:49 UTC (permalink / raw) To: Marc Kleine-Budde Cc: kernel test robot, kbuild-all, Linux Kernel Mailing List, Arunachalam Santhanam On Wed. 2 Mar 2022 at 19:32, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 02.03.2022 17:47:08, kernel test robot wrote: > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > head: 575115360652e9920cc56a028a286ebe9bf82694 > > commit: c664e2137a27680922d8aeb64fb10313416b254f can: etas_es58x: add support for the ETAS ES58X_FD CAN USB interfaces > > date: 11 months ago > > compiler: powerpc64-linux-gcc (GCC) 11.2.0 > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot <yujie.liu@intel.com> Do we still need the tag for false positives (c.f. below)? I am fine to add it, but maybe this will mess with your statistics? > > > > cppcheck possible warnings: (new ones prefixed by >>, may not be real problems) Indeed, not a real problem. My wild guess is that cppcheck does not recognize __stringify() as a pre-procesor macro. That would not be the first time a static analyzer would fail on it. checkpatch had the same issue which I fixed in: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b844345fc2a9c46f8bb8cdb7408c766dfcdd83d Or maybe cppcheck does not try to expand the macro and directly yield a warning? Overall, I think that GCC/Clang already does a good job at finding usage of uninitialized variable. If cppcheck is less mature in this aspect, I suggest to deactivate this particular cppcheck rule and leave this job to GCC/Clang's -Wuninitialized. > > In file included from drivers/net/can/usb/etas_es58x/es58x_fd.c: > > >> drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] > > ret = es58x_check_msg_len(es58x_dev->dev, *rx_event_msg, msg_len); > > ^ > > > > vim +174 drivers/net/can/usb/etas_es58x/es58x_fd.c > > > > c664e2137a2768 Vincent Mailhol 2021-04-10 165 > > c664e2137a2768 Vincent Mailhol 2021-04-10 166 static int es58x_fd_rx_event_msg(struct net_device *netdev, > > c664e2137a2768 Vincent Mailhol 2021-04-10 167 const struct es58x_fd_urb_cmd *es58x_fd_urb_cmd) > > c664e2137a2768 Vincent Mailhol 2021-04-10 168 { > > c664e2137a2768 Vincent Mailhol 2021-04-10 169 struct es58x_device *es58x_dev = es58x_priv(netdev)->es58x_dev; > > c664e2137a2768 Vincent Mailhol 2021-04-10 170 u16 msg_len = get_unaligned_le16(&es58x_fd_urb_cmd->msg_len); > > c664e2137a2768 Vincent Mailhol 2021-04-10 @171 const struct es58x_fd_rx_event_msg *rx_event_msg; > > c664e2137a2768 Vincent Mailhol 2021-04-10 172 int ret; > > c664e2137a2768 Vincent Mailhol 2021-04-10 173 > > c664e2137a2768 Vincent Mailhol 2021-04-10 @174 ret = es58x_check_msg_len(es58x_dev->dev, *rx_event_msg, msg_len); > > c664e2137a2768 Vincent Mailhol 2021-04-10 175 if (ret) > > c664e2137a2768 Vincent Mailhol 2021-04-10 176 return ret; > > c664e2137a2768 Vincent Mailhol 2021-04-10 177 > > c664e2137a2768 Vincent Mailhol 2021-04-10 178 rx_event_msg = &es58x_fd_urb_cmd->rx_event_msg; > > c664e2137a2768 Vincent Mailhol 2021-04-10 179 > > c664e2137a2768 Vincent Mailhol 2021-04-10 180 return es58x_rx_err_msg(netdev, rx_event_msg->error_code, > > c664e2137a2768 Vincent Mailhol 2021-04-10 181 rx_event_msg->event_code, > > c664e2137a2768 Vincent Mailhol 2021-04-10 182 get_unaligned_le64(&rx_event_msg->timestamp)); > > c664e2137a2768 Vincent Mailhol 2021-04-10 183 } > > c664e2137a2768 Vincent Mailhol 2021-04-10 184 > > Thanks for the report. > > This looks like a false positive to me, as es58x_check_msg_len() is not > a function, but a macro: > > | #define es58x_check_msg_len(dev, msg, actual_len) \ > | __es58x_check_msg_len(dev, __stringify(msg), \ > | actual_len, sizeof(msg)) > > __es58x_check_msg_len() don't use "rx_event_msg" directly, but only a > string representation of it and a "sizeof()". Ack. > I think it's possible to assign rx_event_msg before the > es58x_check_msg_len(). Yes, I will do so. Even if this is a false positive, this pattern can be misleading. e.g. during a code review, this does indeed look incorrect at first glance. Also, doing such change would be consistent with was is done in other functions: https://elixir.bootlin.com/linux/latest/source/drivers/net/can/usb/etas_es58x/es58x_fd.c#L210 This not being a bug fix, is it fine to send it to net-next? Or do you see a need to backport this? > I think (hope?) the compiler will not optimize > anything away. :) With a function call and a return statement, the compiler would need to be severely defective to try to optimize this away :) > regards, > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] 2022-03-02 12:49 ` Vincent MAILHOL @ 2022-03-02 13:04 ` Marc Kleine-Budde 2022-03-02 14:42 ` Vincent MAILHOL 0 siblings, 1 reply; 6+ messages in thread From: Marc Kleine-Budde @ 2022-03-02 13:04 UTC (permalink / raw) To: Vincent MAILHOL Cc: kernel test robot, kbuild-all, Linux Kernel Mailing List, Arunachalam Santhanam, linux-can [-- Attachment #1: Type: text/plain, Size: 1365 bytes --] On 02.03.2022 21:49:27, Vincent MAILHOL wrote: > > I think it's possible to assign rx_event_msg before the > > es58x_check_msg_len(). > > Yes, I will do so. Even if this is a false positive, this pattern > can be misleading. e.g. during a code review, this does indeed > look incorrect at first glance. > > Also, doing such change would be consistent with was is done in > other functions: > https://elixir.bootlin.com/linux/latest/source/drivers/net/can/usb/etas_es58x/es58x_fd.c#L210 > > This not being a bug fix, is it fine to send it to net-next? ACK > Or do you see a need to backport this? Don't think so. > > I think (hope?) the compiler will not optimize > > anything away. :) > > With a function call and a return statement, the compiler would > need to be severely defective to try to optimize this away :) I was thinking of this: | void *foo = bar->baz; | | if (!bar) | return; | | printf("%p", foo); There were/are compilers that optimize the bar NULL pointer check away, because bar has already been de-referenced. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] 2022-03-02 13:04 ` Marc Kleine-Budde @ 2022-03-02 14:42 ` Vincent MAILHOL 2022-03-02 14:47 ` Marc Kleine-Budde 0 siblings, 1 reply; 6+ messages in thread From: Vincent MAILHOL @ 2022-03-02 14:42 UTC (permalink / raw) To: Marc Kleine-Budde Cc: kernel test robot, kbuild-all, Linux Kernel Mailing List, Arunachalam Santhanam, linux-can On Wed. 2 Mar 2022 at 22:04, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > I was thinking of this: > > | void *foo = bar->baz; > | > | if (!bar) > | return; > | > | printf("%p", foo); > > There were/are compilers that optimize the bar NULL pointer check away, > because bar has already been de-referenced. Sorry, I do not get your example. If bar is NULL, | void *foo = bar->baz; would segfault and thus the check is not reached. If bar is not NULL, the check succeeds. In both cases, the return statement of the if branch is never executed making this some dead code. So I do not see why this is an issue if the compiler removes it. Yours sincerely, Vincent Mailhol ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] 2022-03-02 14:42 ` Vincent MAILHOL @ 2022-03-02 14:47 ` Marc Kleine-Budde 0 siblings, 0 replies; 6+ messages in thread From: Marc Kleine-Budde @ 2022-03-02 14:47 UTC (permalink / raw) To: Vincent MAILHOL Cc: kernel test robot, kbuild-all, Linux Kernel Mailing List, Arunachalam Santhanam, linux-can [-- Attachment #1: Type: text/plain, Size: 1217 bytes --] On 02.03.2022 23:42:53, Vincent MAILHOL wrote: > On Wed. 2 Mar 2022 at 22:04, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > I was thinking of this: > > > > | void *foo = bar->baz; > > | > > | if (!bar) > > | return; > > | > > | printf("%p", foo); > > > > There were/are compilers that optimize the bar NULL pointer check away, > > because bar has already been de-referenced. > > Sorry, I do not get your example. If bar is NULL, > | void *foo = bar->baz; > would segfault and thus the check is not reached. ACK > If bar is not NULL, the check succeeds. > > In both cases, the return statement of the if branch is never > executed making this some dead code. So I do not see why this is > an issue if the compiler removes it. IIRC in some cases the code was shuffled around by the compiler and the NULL pointer check was done....and with a new compiler version it stopped working :) regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-02 14:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <202203021333.mMJpWPzx-lkp@intel.com>
2022-03-02 9:47 ` drivers/net/can/usb/etas_es58x/es58x_fd.c:174:8: warning: Uninitialized variable: rx_event_msg [uninitvar] kernel test robot
2022-03-02 10:32 ` Marc Kleine-Budde
2022-03-02 12:49 ` Vincent MAILHOL
2022-03-02 13:04 ` Marc Kleine-Budde
2022-03-02 14:42 ` Vincent MAILHOL
2022-03-02 14:47 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox