Powered by Linux
Re: uninitialized symbol 'xxxx' is too overzealous? — Semantic Matching Tool

Re: uninitialized symbol 'xxxx' is too overzealous?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On May 25, 2018, at 8:12 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> 
> On Thu, May 17, 2018 at 08:55:04PM -0400, Oleg Drokin wrote:
>> Hello!
>> 
>>   I just updated to the tip of master for smatch and it seems uninitialized variables detection is way too overzealous.
>>   While it did find one valid usage, I cannot stop imagining that it basically assumes that if you
>>   pass a pointer to uninitialized variable somewhere, the variable would remain unitialized at least sometimes.
>> 
>>   E.g. this code:
>> struct page *ll_get_dir_page(struct inode *dir, struct md_op_data *op_data,
>>                             __u64 offset, struct ll_dir_chain *chain)
>> {
>>        struct md_callback      cb_op;
>>        struct page             *page;
>>        int                     rc;
>> 
>>        cb_op.md_blocking_ast = ll_md_blocking_ast;
>>        rc = md_read_page(ll_i2mdexp(dir), op_data, &cb_op, offset, &page);
>>        if (rc != 0)
>>                return ERR_PTR(rc);
>> 
>>        return page;
>> }
>> 
>>   Produces:
>> lustre/llite/dir.c:156 ll_get_dir_page() error: uninitialized symbol 'page’.
> 
> I don't get this warning on my tree.  I pushed some changes 11 days ago
> which we possibly related to this so maybe that made a difference?

I was on 2f66d40cbf57b0bd581fe75447d2a8625fc7bb1d which is dated
May 15th and shows as current master for me.

> Can you run `smatch_data/db/smdb.py return_states md_read_page` from
> the top level directory?  It should mark the parameter as an
> UNTRACKED_PARAM:
> 
> drivers/staging/lustre/lustre/lmv/lmv_obd.c | md_read_page | 1191 | s32min-s32max | UNTRACKED_PARAM |   4 |                    $ |                      |

Ah, you are looking into the in-kernel staging driver, I am talking about the external tree code, and they did diverge.

Though a similar line is present for me:
.../git/lustre-release/lustre/lmv/lmv_obd.c | md_read_page | 1975 | s32min-s32max | UNTRACKED_PARAM |   4 |                    $ |                      |

> You can just rebuild part of the DB to test this:
> 
> kchecker --info drivers/staging/lustre/lustre/lmv/lmv_obd.c > warns.txt
> ~/path/to/smatch_data/db/reload_partial.sh warns.txt
> 
> 
>>   I went through everythign down the stack and I cannot see how.
>> 
>>   The actual code could be seen here:
>> https://git.hpdd.intel.com/?p=fs/lustre-release.git;a=blob;f=lustre/llite/dir.c;h=44b174726677b780f01c4bd252d599deb08ac083;hb=refs/heads/master
>> 
>>   Am I missing something? This is on by default even without —-spammy
>> 
>>   Also is —two-pass supposed to do anything worthwhile? So far the most visible effect is complaining
>>   how basically every function call return is “unused”.
> 
> No.  --two-passes isn't useful.  It was supposed to warn about unused
> returns but I haven't been testing it so I didn't notice when it broke.
> In theory, --two-passes is the future because it would make parsing
> loops a lot better.  But the problem is it makes everything twice as
> slow…

Ok, good to know.

> 
> regards,
> dan carpenter
> 
> --
> To unsubscribe from this list: send the line "unsubscribe smatch" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux