REGRESSION: implied: we have to make the false states match as well

[conchur@xxxxxx]

Hi,

I've noticed a regression with the commit
0b78084aeb0004db657d6e2d4fa4c17ec9f2e8e5 ("implied: we have to make the
false states match as well"). Now I get multiple false positives when
compiling the kernel with 

make CHECK="smatch -p=kernel --two-passes --file-output"CC="cgcc"

    net/batman-adv/soft-interface.c:894 batadv_softif_slave_add warn: unused return: ret = batadv_hardif_enable_interface()

This would be this function which definitely uses the return

    static int batadv_softif_slave_add(struct net_device *dev,
    				   struct net_device *slave_dev)
    {
    	struct batadv_hard_iface *hard_iface;
    	int ret = -EINVAL;

    	hard_iface = batadv_hardif_get_by_netdev(slave_dev);
    	if (!hard_iface || hard_iface->soft_iface)
    		goto out;

    	ret = batadv_hardif_enable_interface(hard_iface, dev->name);

    out:
    	if (hard_iface)
    		batadv_hardif_put(hard_iface);
    	return ret;
    }

Or:

    net/batman-adv/bat_iv_ogm.c:1481 batadv_iv_ogm_process_per_outif warn: unused return: router_router = batadv_orig_router_get()

which would be this snippet:

    	router = batadv_orig_router_get(orig_node, if_outgoing);
    	if (router) {
    		router_router = batadv_orig_router_get(router->orig_node,
    						       if_outgoing);
    		router_ifinfo = batadv_neigh_ifinfo_get(router, if_outgoing);
    	}
    
    	if ((router_ifinfo && router_ifinfo->bat_iv.tq_avg != 0) &&
    	    (batadv_compare_eth(router->addr, ethhdr->h_source)))
    		is_from_best_next_hop = true;
    
    	prev_sender = ogm_packet->prev_sender;
    	/* avoid temporary routing loops */
    	if (router && router_router &&
    	    (batadv_compare_eth(router->addr, prev_sender)) &&
    	    !(batadv_compare_eth(ogm_packet->orig, prev_sender)) &&
    	    (batadv_compare_eth(router->addr, router_router->addr))) {
    		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
    			   "Drop packet: ignoring all rebroadcast packets that may make me loop (sender: %pM)\n",
    			   ethhdr->h_source);
    		goto out;
    	}


These are rather hard to test because they are inside the kernel. I've
prepared minified versions of the functions which can be tested without
the kernel overhead:

Here is the minified versions of the first function:

    cat << EOF > test.c
    struct bar {
    	void *y;
    };

    struct bar *getit(void);
    int doit(struct bar *x);
    void putit(struct bar *x);

    int foo(void)
    {
    	int ret = 0;
    	struct bar *x;

    	x = getit();
    	if (!x || x->y)
    		goto out;

    	ret = doit(x);

    out:
    	if (x)
    		putit(x);

    	return ret;
    }
    EOF
    smatch -p=kernel --two-passes test.c

The problem seems to disappear when removing the "|| x->y".

The minified version of the second function:

    cat << EOF > test.c
    void *getit(void);
    void doit(void);

    void foo(void)
    {
    	void *a = (void *)0;
    	void *b = (void *)0;
    	void *c = (void *)0;
    	int d = 0;

    	a = getit();
    	if (a) {
    		b = getit();
    		c = getit();
    	}

    	if (c)
    		d = 1;

    	if (a && b && c)
    		doit();

    	if (d)
    		doit();
    }
    EOF
    smatch -p=kernel --two-passes test.c

It doesn't print the warning when removing the "if (c)" + the next line
--
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