On Wed, Oct 18, 2023 at 09:19:42PM -0400, Kent Overstreet wrote:
> On Fri, Oct 13, 2023 at 10:17:21AM +0300, Dan Carpenter wrote:
> > Hello Kent Overstreet,
> >
> > The patch e0750d947352: "bcachefs: Initial commit" from Mar 16, 2017
> > (linux-next), leads to the following Smatch static checker warning:
> >
> > fs/bcachefs/btree_update_interior.c:2265 bch2_btree_node_update_key()
> > error: 'new_hash' dereferencing possible ERR_PTR()
> >
> > fs/bcachefs/btree_update_interior.c
> > 2227 int bch2_btree_node_update_key(struct btree_trans *trans, struct
> > btree_iter *iter,
> > 2228 struct btree *b, struct bkey_i
> > *new_key,
> > 2229 unsigned commit_flags, bool
> > skip_triggers)
> > 2230 {
> > 2231 struct bch_fs *c = trans->c;
> > 2232 struct btree *new_hash = NULL;
> > 2233 struct btree_path *path = iter->path;
> > 2234 struct closure cl;
> > 2235 int ret = 0;
> > 2236
> > 2237 ret = bch2_btree_path_upgrade(trans, path, b->c.level + 1);
> > 2238 if (ret)
> > 2239 return ret;
> > 2240
> > 2241 closure_init_stack(&cl);
> > 2242
> > 2243 /*
> > 2244 * check btree_ptr_hash_val() after @b is locked by
> > 2245 * btree_iter_traverse():
> > 2246 */
> > 2247 if (btree_ptr_hash_val(new_key) != b->hash_val) {
> > 2248 ret = bch2_btree_cache_cannibalize_lock(c, &cl);
> > 2249 if (ret) {
> > 2250 ret = drop_locks_do(trans,
> > (closure_sync(&cl), 0));
> > 2251 if (ret)
> > 2252 return ret;
> > 2253 }
> > 2254
> > 2255 new_hash = bch2_btree_node_mem_alloc(trans, false);
> >
> > This allocation can fail and it returns an error pointer?
>
> It doesn't fail when we hold the cannibalize lock. I can add an
> assertion that documents that if it'll stop smatch from complaining...
Nah. It won't stop Smatch from complaining unless it has a panic().
Smatch ignores assertions which only trigger a warning.
Just leave it. Generally the policy with Smatch is to only report
warnings once and search lore for this email if it's a false positive.
regards,
dan carpenter
