* re: mmc: rtsx: add support for pre_req and post_req
@ 2014-02-24 22:03 Dan Carpenter
2014-02-25 1:52 ` micky
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2014-02-24 22:03 UTC (permalink / raw)
To: micky_ching; +Cc: linux-mmc
Hello Micky Ching,
This is a semi-automatic email about new static checker warnings.
The patch c42deffd5b53: "mmc: rtsx: add support for pre_req and
post_req" from Feb 17, 2014, leads to the following Smatch complaint:
drivers/mmc/host/rtsx_pci_sdmmc.c:194 sd_finish_request()
error: we previously assumed 'mrq' could be null (see line 158)
drivers/mmc/host/rtsx_pci_sdmmc.c
157 mrq = host->mrq;
158 if (!mrq) {
^^^^
Patch introduces check.
159 dev_err(sdmmc_dev(host), "error: no request need finish\n");
160 goto out;
161 }
162
163 cmd = mrq->cmd;
164 data = mrq->data;
165
166 any_error = (mrq->sbc && mrq->sbc->error) ||
167 (mrq->stop && mrq->stop->error) ||
168 (cmd && cmd->error) || (data && data->error);
169
170 if (any_error) {
171 rtsx_pci_stop_cmd(pcr);
172 sd_clear_error(host);
173 }
174
175 if (data) {
176 if (any_error)
177 data->bytes_xfered = 0;
178 else
179 data->bytes_xfered = data->blocks * data->blksz;
180
181 if (!data->host_cookie)
182 rtsx_pci_dma_unmap_sg(pcr, data->sg, data->sg_len,
183 data->flags & MMC_DATA_READ);
184
185 }
186
187 host->mrq = NULL;
188 host->cmd = NULL;
189 host->data = NULL;
190
191 out:
192 spin_unlock_irqrestore(&host->lock, flags);
193 mutex_unlock(&pcr->pcr_mutex);
194 mmc_request_done(host->mmc, mrq);
^^^
Dereferenced inside function. Patch introduces dereference.
195 }
196
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: mmc: rtsx: add support for pre_req and post_req
2014-02-24 22:03 mmc: rtsx: add support for pre_req and post_req Dan Carpenter
@ 2014-02-25 1:52 ` micky
2014-02-25 7:59 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: micky @ 2014-02-25 1:52 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mmc
Hi Dan,
I'm wondering how to get this warning info, so I can check before
sending patch.
Can you give me some idea?
Best Regards.
micky.
On 02/25/2014 06:03 AM, Dan Carpenter wrote:
> Hello Micky Ching,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch c42deffd5b53: "mmc: rtsx: add support for pre_req and
> post_req" from Feb 17, 2014, leads to the following Smatch complaint:
>
> drivers/mmc/host/rtsx_pci_sdmmc.c:194 sd_finish_request()
> error: we previously assumed 'mrq' could be null (see line 158)
> drivers/mmc/host/rtsx_pci_sdmmc.c
> 157 mrq = host->mrq;
> 158 if (!mrq) {
> ^^^^
> Patch introduces check.
>
> 159 dev_err(sdmmc_dev(host), "error: no request need finish\n");
> 160 goto out;
> 161 }
> 162
> 163 cmd = mrq->cmd;
> 164 data = mrq->data;
> 165
> 166 any_error = (mrq->sbc && mrq->sbc->error) ||
> 167 (mrq->stop && mrq->stop->error) ||
> 168 (cmd && cmd->error) || (data && data->error);
> 169
> 170 if (any_error) {
> 171 rtsx_pci_stop_cmd(pcr);
> 172 sd_clear_error(host);
> 173 }
> 174
> 175 if (data) {
> 176 if (any_error)
> 177 data->bytes_xfered = 0;
> 178 else
> 179 data->bytes_xfered = data->blocks * data->blksz;
> 180
> 181 if (!data->host_cookie)
> 182 rtsx_pci_dma_unmap_sg(pcr, data->sg, data->sg_len,
> 183 data->flags & MMC_DATA_READ);
> 184
> 185 }
> 186
> 187 host->mrq = NULL;
> 188 host->cmd = NULL;
> 189 host->data = NULL;
> 190
> 191 out:
> 192 spin_unlock_irqrestore(&host->lock, flags);
> 193 mutex_unlock(&pcr->pcr_mutex);
> 194 mmc_request_done(host->mmc, mrq);
> ^^^
> Dereferenced inside function. Patch introduces dereference.
>
> 195 }
> 196
>
> regards,
> dan carpenter
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: mmc: rtsx: add support for pre_req and post_req
2014-02-25 1:52 ` micky
@ 2014-02-25 7:59 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-02-25 7:59 UTC (permalink / raw)
To: micky; +Cc: linux-mmc
On Tue, Feb 25, 2014 at 09:52:12AM +0800, micky wrote:
> Hi Dan,
>
> I'm wondering how to get this warning info, so I can check before
> sending patch.
> Can you give me some idea?
>
This is a smatch thing, but you it's a cross function bug so you have to
build the database first and that takes some hours. The commands are
simple though. Run:
~/path/to/smatch_scripts/build_kernel_data.sh
~/path/to/smatch_scripts/kchecker drivers/mmc/host/rtsx_pci_sdmmc.c
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* re: mmc: rtsx: add support for pre_req and post_req
@ 2014-02-24 22:03 Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-02-24 22:03 UTC (permalink / raw)
To: micky_ching; +Cc: linux-mmc
Hello Micky Ching,
This is a semi-automatic email about new static checker warnings.
The patch c42deffd5b53: "mmc: rtsx: add support for pre_req and
post_req" from Feb 17, 2014, leads to the following Smatch complaint:
drivers/mmc/host/rtsx_pci_sdmmc.c:504 sd_get_rsp()
error: we previously assumed 'cmd' could be null (see line 434)
drivers/mmc/host/rtsx_pci_sdmmc.c
433
434 if (!cmd) {
^^^
Patch introduces check.
435 dev_err(sdmmc_dev(host), "error: cmd not exist\n");
436 goto out;
437 }
438
[ snip ]
497 if (cmd->data) {
498 sd_start_multi_rw(host, host->mrq);
499 spin_unlock_irqrestore(&host->lock, flags);
500 return;
501 }
502
503 out:
504 cmd->error = err;
^^^^^^^^^^
Dereference was already there.
505
506 tasklet_schedule(&host->finish_tasklet);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* re: mmc: rtsx: add support for pre_req and post_req
@ 2014-02-28 20:35 Dan Carpenter
2014-03-03 1:13 ` micky
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2014-02-28 20:35 UTC (permalink / raw)
To: micky_ching; +Cc: linux-mmc
Hello Micky Ching,
The patch c42deffd5b53: "mmc: rtsx: add support for pre_req and
post_req" from Feb 17, 2014, leads to the following static checker
warning:
drivers/mmc/host/rtsx_pci_sdmmc.c:525 sd_pre_dma_transfer()
warn: we tested 'next' before and it was 'false'
drivers/mmc/host/rtsx_pci_sdmmc.c
520 "error: invalid cookie data[%d] host[%d]\n",
521 data->host_cookie, host->next_data.cookie);
522 data->host_cookie = 0;
523 }
524
525 if (next || (!next && data->host_cookie != host->next_data.cookie))
^^^^^
We are testing "next" twice. Often that means the intention was to test
a different variable?
526 sg_count = rtsx_pci_dma_map_sg(pcr,
527 data->sg, data->sg_len, read);
528 else
529 sg_count = host->next_data.sg_count;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: mmc: rtsx: add support for pre_req and post_req
2014-02-28 20:35 Dan Carpenter
@ 2014-03-03 1:13 ` micky
0 siblings, 0 replies; 6+ messages in thread
From: micky @ 2014-03-03 1:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mmc
On 03/01/2014 04:35 AM, Dan Carpenter wrote:
> Hello Micky Ching,
>
> The patch c42deffd5b53: "mmc: rtsx: add support for pre_req and
> post_req" from Feb 17, 2014, leads to the following static checker
> warning:
>
> drivers/mmc/host/rtsx_pci_sdmmc.c:525 sd_pre_dma_transfer()
> warn: we tested 'next' before and it was 'false'
>
> drivers/mmc/host/rtsx_pci_sdmmc.c
> 520 "error: invalid cookie data[%d] host[%d]\n",
> 521 data->host_cookie, host->next_data.cookie);
> 522 data->host_cookie = 0;
> 523 }
> 524
> 525 if (next || (!next && data->host_cookie != host->next_data.cookie))
> ^^^^^
> We are testing "next" twice. Often that means the intention was to test
> a different variable?
Yes, it can be write as follow.
if (next || data->host_cookie != host->next_data.cookie)
>
> 526 sg_count = rtsx_pci_dma_map_sg(pcr,
> 527 data->sg, data->sg_len, read);
> 528 else
> 529 sg_count = host->next_data.sg_count;
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-03 1:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-24 22:03 mmc: rtsx: add support for pre_req and post_req Dan Carpenter
2014-02-25 1:52 ` micky
2014-02-25 7:59 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2014-02-24 22:03 Dan Carpenter
2014-02-28 20:35 Dan Carpenter
2014-03-03 1:13 ` micky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).