[Cocci] Searching for duplicate statements in if branches (with SmPL)?

* [Cocci] Searching for duplicate statements in if branches (with SmPL)?
@ 2019-04-11 15:23 Markus Elfring
       [not found] ` <alpine.DEB.2.20.1904111728380.3143@hadrien>
  0 siblings, 1 reply; 19+ messages in thread
From: Markus Elfring @ 2019-04-11 15:23 UTC (permalink / raw)
  To: Coccinelle

Hello,

I have constructed the following small SmPL script.


@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if (...)
*{
    ... when any
*   s1
*   s2
*}
 ... when any
*if (...)
*{
    ... when any
*   s1
*   s2
*}
 ... when any
 }


Test result:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/ti/netcp_ethss.c?id=75eac7b5f68b0a0671e795ac636457ee27cc11d8#n3562
https://elixir.bootlin.com/linux/v5.1-rc4/source/drivers/net/ethernet/ti/netcp_ethss.c#L3562

@@ -1829,11 +1829,7 @@ static void gbe_reset_mod_stats(struct g
 	int i;

 	for (i = 0; i < gbe_dev->num_et_stats; i++) {
-		if (gbe_dev->et_stats[i].type == stats_mod) {
 			p_stats_entry = base + gbe_dev->et_stats[i].offset;
-			gbe_dev->hw_stats[i] = 0;
-			gbe_dev->hw_stats_prev[i] = readl(p_stats_entry);
-		}
 	}
 }

@@ -2808,10 +2804,6 @@ static int gbe_set_rx_mode(void *intf_pr
 	cpsw_ale_control_set(ale, 0, ALE_AGEOUT, 1);
 	do {
 		cpu_relax();
-		if (cpsw_ale_control_get(ale, 0, ALE_AGEOUT)) {
-			ret = 0;
-			break;
-		}

 	} while (time_after(timeout, jiffies));

…
@@ -3657,31 +3636,14 @@ static int gbe_probe(struct netcp_device

 	ret = netcp_txpipe_init(&gbe_dev->tx_pipe, netcp_device,
 				gbe_dev->dma_chan_name, gbe_dev->tx_queue_id);
-	if (ret) {
-		of_node_put(interfaces);
-		return ret;
-	}

 	ret = netcp_txpipe_open(&gbe_dev->tx_pipe);
-	if (ret) {
-		of_node_put(interfaces);
-		return ret;
-	}

 	/* Create network interfaces */
 	INIT_LIST_HEAD(&gbe_dev->gbe_intf_head);
 	for_each_child_of_node(interfaces, interface) {
 		ret = of_property_read_u32(interface, "slave-port", &slave_num);
-		if (ret) {
-			dev_err(dev, "missing slave-port parameter, skipping interface configuration for %pOFn\n",
-				interface);
-			continue;
-		}
 		gbe_dev->num_slaves++;
-		if (gbe_dev->num_slaves >= gbe_dev->max_num_slaves) {
-			of_node_put(interface);
-			break;
-		}
 	}
 	of_node_put(interfaces);



Now I wonder again why the first two diff hunks are presented by this
source code search approach. It seems that the hits for the functions
“gbe_reset_mod_stats” and “gbe_set_rx_mode” are false positives
(because only one if statement is marked there).

The last diff hunk points an implementation detail out which is discussed
for the update suggestion “ethernet: ti: eliminate a bit of duplicate code
in gbe_probe()”.
https://lkml.org/lkml/2019/4/9/979
https://lore.kernel.org/patchwork/patch/1059888/
https://lore.kernel.org/lkml/1554864935-8299-1-git-send-email-wen.yang99@zte.com.cn/

But it shows also source code places where the SmPL specification is
too generic so far.


Another SmPL script variant presents also a result for additional considerations.


@duplicated_code@
identifier work;
statement s1, s2, es;
type T;
@@
 T work(...)
 {
 ... when any
*if (...)
*{
    ... when any
*   s1
*   s2
*}
 else
 es
 ... when any
*if (...)
*{
    ... when any
*   s1
*   s2
*}
 else
 es
 ... when any
 }


@@ -3205,14 +3205,8 @@ static void init_secondary_ports(struct
 				       slave->phy_node,
 				       gbe_adjust_link_sec_slaves,
 				       0, phy_mode);
-		if (!slave->phy) {
 			dev_err(dev, "phy not found for slave %d\n",
 				slave->slave_num);
-		} else {
-			dev_dbg(dev, "phy found: id is: 0x%s\n",
-				phydev_name(slave->phy));
-			phy_start(slave->phy);
-		}
 	}
 }


I am curious if better solutions can be found for such software.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 19+ messages in thread