diff mbox

[dpdk-dev] bond: fix for mac assignment to slaves device

Message ID F6F2A6264E145F47A18AB6DF8E87425D12B67417@IRSMSX102.ger.corp.intel.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Wodkowski, PawelX Dec. 8, 2014, 9:21 a.m. UTC 
  > +			memmove(&internals->slaves[i], &internals-
> >slaves[i+1],
> +					(sizeof(internals->slaves[0]) *
                                                                         ^^^
> +					internals->slave_count - i - 1));
                                                                                                                              ^^^

I think that this was not your intention.

I also think that that whole slave_remove() is a little obfuscated. You are using 'found'
variable that is suggesting that there is situation when slave id might be not in slaves array.
You can do something like this

Comments

Wodkowski, PawelX Dec. 8, 2014, 9:31 a.m. UTC | #1 
Some formatting issues during posting. I was talking about parenthesis in 
count calculation

 (sizeof(internals->slaves[0]) *
            internals->slave_count - i - 1));
diff mbox

Patch

diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index bb2b909..cfa244d 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -982,16 +982,16 @@ 
 slave_remove(struct bond_dev_private *internals,
 		struct rte_eth_dev *slave_eth_dev)
 {
-	int i, found = 0;
+	uint8_t i;
 
 	for (i = 0; i < internals->slave_count; i++) {
 		if (internals->slaves[i].port_id ==	slave_eth_dev->data->port_id)
-			found = 1;
-
-		if (found && i < (internals->slave_count - 1))
-			memcpy(&internals->slaves[i], &internals->slaves[i+1],
-					sizeof(internals->slaves[i]));
+			break;
 	}
+
+	if (i != internals->slave_count)
+		memmove(&internals->slaves[i], &internals->slaves[i + 1],
+			sizeof(internals->slaves[0]) * (internals->slave_count - i - 1));
 
 	internals->slave_count--;
 }
@@ -1501,6 +1501,8 @@