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?

    2256         }
    2257 
    2258         path->intent_ref++;
    2259         ret = __bch2_btree_node_update_key(trans, iter, b, new_hash, 
new_key,
    2260                                            commit_flags, 
skip_triggers);
    2261         --path->intent_ref;
    2262 
    2263         if (new_hash) {
    2264                 mutex_lock(&c->btree_cache.lock);
--> 2265                 list_move(&new_hash->list, &c->btree_cache.freeable);
    2266                 mutex_unlock(&c->btree_cache.lock);
    2267 
    2268                 six_unlock_write(&new_hash->c.lock);
    2269                 six_unlock_intent(&new_hash->c.lock);
    2270         }
    2271         closure_sync(&cl);
    2272         bch2_btree_cache_cannibalize_unlock(c);
    2273         return ret;
    2274 }

regards,
dan carpenter

Reply via email to