From: Dan Carpenter <dan.carpenter@linaro.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: oe-kbuild@lists.linux.dev,
Md Sadre Alam <quic_mdalam@quicinc.com>,
lkp@intel.com, oe-kbuild-all@lists.linux.dev,
Linux Memory Management List <linux-mm@kvack.org>,
Sricharan Ramabadhran <quic_srichara@quicinc.com>,
Manivannan Sadhasivam <mani@kernel.org>
Subject: Re: [linux-next:master 1937/6910] drivers/mtd/nand/raw/qcom_nandc.c:2941 qcom_op_cmd_mapping() error: uninitialized symbol 'ret'.
Date: Fri, 4 Aug 2023 19:52:50 +0300 [thread overview]
Message-ID: <3604e2ed-8d30-4f7b-9e56-7af5b23b2ac5@kadam.mountain> (raw)
In-Reply-To: <20230804184550.0cb12369@xps-13>
On Fri, Aug 04, 2023 at 06:45:50PM +0200, Miquel Raynal wrote:
> Hi Sadre, Sricharan & Manivannan,
>
> A couple of questions for you below.
>
> dan.carpenter@linaro.org wrote on Thu, 3 Aug 2023 15:20:56 +0300:
>
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head: fb4327106e5250ee360d0d8b056c1eef7eeb9a98
> > commit: 89550beb098e04b951df079483fb064eafc0e5fa [1937/6910] mtd: rawnand: qcom: Implement exec_op()
> > config: riscv-randconfig-m031-20230730 (https://download.01.org/0day-ci/archive/20230803/202308032022.SnXkKyFs-lkp@intel.com/config)
> > compiler: riscv64-linux-gcc (GCC) 12.3.0
> > reproduce: (https://download.01.org/0day-ci/archive/20230803/202308032022.SnXkKyFs-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > | Closes: https://lore.kernel.org/r/202308032022.SnXkKyFs-lkp@intel.com/
> >
> > New smatch warnings:
> > drivers/mtd/nand/raw/qcom_nandc.c:2941 qcom_op_cmd_mapping() error: uninitialized symbol 'ret'.
> > drivers/mtd/nand/raw/qcom_nandc.c:3369 qcom_check_op() warn: was && intended here instead of ||?
> >
> > Old smatch warnings:
> > drivers/mtd/nand/raw/qcom_nandc.c:3076 qcom_read_status_exec() warn: inconsistent indenting
> >
> > vim +/ret +2941 drivers/mtd/nand/raw/qcom_nandc.c
> >
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2909 static int qcom_op_cmd_mapping(struct qcom_nand_controller *nandc, u8 cmd,
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2910 struct qcom_op *q_op)
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2911 {
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2912 int ret;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2913
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2914 switch (cmd) {
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2915 case NAND_CMD_RESET:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2916 ret = OP_RESET_DEVICE;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2917 break;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2918 case NAND_CMD_READID:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2919 ret = OP_FETCH_ID;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2920 break;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2921 case NAND_CMD_PARAM:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2922 if (nandc->props->qpic_v2)
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2923 ret = OP_PAGE_READ_ONFI_READ;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2924 else
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2925 ret = OP_PAGE_READ;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2926 break;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2927 case NAND_CMD_ERASE1:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2928 case NAND_CMD_ERASE2:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2929 ret = OP_BLOCK_ERASE;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2930 break;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2931 case NAND_CMD_STATUS:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2932 ret = OP_CHECK_STATUS;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2933 break;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2934 case NAND_CMD_PAGEPROG:
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2935 ret = OP_PROGRAM_PAGE;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2936 q_op->flag = OP_PROGRAM_PAGE;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2937 nandc->exec_opwrite = true;
> > 89550beb098e04 drivers/mtd/nand/raw/qcom_nandc.c Md Sadre Alam 2023-07-10 2938 break;
> >
> > No default case. I'm more concerned about the && vs || warning, but
> > the zero day bot doesn't include that code into the email.
>
> The default case here is in theory not possible, as long as
> qcom_check_op() is properly implemented. We should however silence the
> warning by handling it. Maybe a WARN_ON_ONCE() + ret = <anything that
> will produce an error when executing the command> would work.
>
The kbuild-bot runs without the cross function database so it has a lot
of false positives that you wouldn't see if you had the DB. (There
isn't really a way to build with the cross function DB. It takes way
way way too long. Also it sometimes finds bug which are complicated to
explain or to assign blame for in an automated way). But these false
positives are frustrating to me.
In real life, GCC is going to automatically initialize stack variables
to zero on production systems.
> However I doubt the following piece of code has been successfully
> tested:
>
> for (op_id = 0; op_id < op->ninstrs; op_id++) {
> instr = &op->instrs[op_id];
>
> switch (instr->type) {
> case NAND_OP_CMD_INSTR:
> if (instr->ctx.cmd.opcode != NAND_CMD_RESET ||
> instr->ctx.cmd.opcode != NAND_CMD_READID ||
> instr->ctx.cmd.opcode != NAND_CMD_PARAM ||
> instr->ctx.cmd.opcode != NAND_CMD_ERASE1 ||
> instr->ctx.cmd.opcode != NAND_CMD_ERASE2 ||
> instr->ctx.cmd.opcode != NAND_CMD_STATUS ||
> instr->ctx.cmd.opcode != NAND_CMD_PAGEPROG)
> return -ENOTSUPP;
> break;
>
> The || should be &&, otherwise it cannot work, or am I missing
> something?
Yeah. That's how this bug normally looks like. NAND_OP_CMD_INSTR
always returns -ENOTSUPP.
regards,
dan carpenter
next prev parent reply other threads:[~2023-08-04 16:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 12:20 Dan Carpenter
2023-08-04 16:45 ` Miquel Raynal
2023-08-04 16:52 ` Dan Carpenter [this message]
2023-08-04 17:07 ` Miquel Raynal
2023-08-05 6:55 ` Manivannan Sadhasivam
2023-08-06 7:58 ` Sricharan Ramabadhran
2023-08-07 18:54 ` Sricharan Ramabadhran
2023-08-07 19:14 ` Miquel Raynal
2023-08-08 5:16 ` Sricharan Ramabadhran
2023-08-18 13:33 ` Miquel Raynal
2023-08-18 13:51 ` Sricharan Ramabadhran
2023-08-18 14:03 ` Miquel Raynal
2023-08-18 14:17 ` Sricharan Ramabadhran
2023-08-18 15:24 ` Sricharan Ramabadhran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3604e2ed-8d30-4f7b-9e56-7af5b23b2ac5@kadam.mountain \
--to=dan.carpenter@linaro.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=mani@kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
--cc=quic_mdalam@quicinc.com \
--cc=quic_srichara@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox