bonding: clean up style for bond_3ad.c
[ This isn't really the patch that introduced the bug. I'm not sure
why Smatch is only complaining about this now. Anyway, it looks buggy. -dan ]
Hello Veaceslav Falico,
This is a semi-automatic email about new static checker warnings.
The patch 3bf2d28a2d71: "bonding: clean up style for bond_3ad.c" from Jan 8, 2014, leads to the following Smatch complaint:
drivers/net/bonding/bond_3ad.c:1346 ad_port_selection_logic()
error: we previously assumed 'port->aggregator' could be null (see line 1211)
drivers/net/bonding/bond_3ad.c
1210 /* if the port is connected to other aggregator, detach it */
1211 if (port->aggregator) {
^^^^^^^^^^^^^^^^Here.
1212 /* detach the port from its former aggregator */
1213 temp_aggregator = port->aggregator;
1214 for (curr_port = temp_aggregator->lag_ports; curr_port;
1215 last_port = curr_port,
1216 curr_port = curr_port->next_port_in_aggregator) {
1217 if (curr_port = port) {
1218 temp_aggregator->num_of_ports--;
1219 /* if it is the first port attached to the
1220 * aggregator
1221 */
1222 if (!last_port) {
1223 temp_aggregator->lag_ports 1224 port->next_port_in_aggregator;
1225 } else {
1226 /* not the first port attached to the
1227 * aggregator
1228 */
1229 last_port->next_port_in_aggregator 1230 port->next_port_in_aggregator;
1231 }
1232
1233 /* clear the port's relations to this
1234 * aggregator
1235 */
1236 port->aggregator = NULL;
1237 port->next_port_in_aggregator = NULL;
1238 port->actor_port_aggregator_identifier = 0;
1239
1240 netdev_dbg(bond->dev, "Port %d left LAG %d\n",
1241 port->actor_port_number,
1242 temp_aggregator->aggregator_identifier);
1243 /* if the aggregator is empty, clear its
1244 * parameters, and set it ready to be attached
1245 */
1246 if (!temp_aggregator->lag_ports)
1247 ad_clear_agg(temp_aggregator);
1248 break;
1249 }
1250 }
1251 if (!curr_port) {
1252 /* meaning: the port was related to an aggregator
1253 * but was not on the aggregator port list
1254 */
1255 net_warn_ratelimited("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
1256 port->slave->bond->dev->name,
1257 port->actor_port_number,
1258 port->slave->dev->name,
1259 port->aggregator->aggregator_identifier);
1260 }
1261 }
1262 /* search on all aggregators for a suitable aggregator for this port */
1263 bond_for_each_slave(bond, slave, iter) {
1264 aggregator = &(SLAVE_AD_INFO(slave)->aggregator);
1265
1266 /* keep a free aggregator for later use(if needed) */
1267 if (!aggregator->lag_ports) {
1268 if (!free_aggregator)
1269 free_aggregator = aggregator;
1270 continue;
1271 }
1272 /* check if current aggregator suits us */
1273 if (((aggregator->actor_oper_aggregator_key = port->actor_oper_port_key) && /* if all parameters match AND */
1274 MAC_ADDRESS_EQUAL(&(aggregator->partner_system), &(port->partner_oper.system)) &&
1275 (aggregator->partner_system_priority = port->partner_oper.system_priority) &&
1276 (aggregator->partner_oper_aggregator_key = port->partner_oper.key)
1277 ) &&
1278 ((!MAC_ADDRESS_EQUAL(&(port->partner_oper.system), &(null_mac_addr)) && /* partner answers */
1279 !aggregator->is_individual) /* but is not individual OR */
1280 )
1281 ) {
1282 /* attach to the founded aggregator */
1283 port->aggregator = aggregator;
1284 port->actor_port_aggregator_identifier 1285 port->aggregator->aggregator_identifier;
1286 port->next_port_in_aggregator = aggregator->lag_ports;
1287 port->aggregator->num_of_ports++;
1288 aggregator->lag_ports = port;
1289 netdev_dbg(bond->dev, "Port %d joined LAG %d(existing LAG)\n",
1290 port->actor_port_number,
1291 port->aggregator->aggregator_identifier);
1292
1293 /* mark this port as selected */
1294 port->sm_vars |= AD_PORT_SELECTED;
1295 found = 1;
1296 break;
1297 }
1298 }
1299
1300 /* the port couldn't find an aggregator - attach it to a new
1301 * aggregator
1302 */
1303 if (!found) {
1304 if (free_aggregator) {
1305 /* assign port a new aggregator */
1306 port->aggregator = free_aggregator;
1307 port->actor_port_aggregator_identifier 1308 port->aggregator->aggregator_identifier;
1309
1310 /* update the new aggregator's parameters
1311 * if port was responsed from the end-user
1312 */
1313 if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)
1314 /* if port is full duplex */
1315 port->aggregator->is_individual = false;
1316 else
1317 port->aggregator->is_individual = true;
1318
1319 port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
1320 port->aggregator->actor_oper_aggregator_key = port->actor_oper_port_key;
1321 port->aggregator->partner_system 1322 port->partner_oper.system;
1323 port->aggregator->partner_system_priority 1324 port->partner_oper.system_priority;
1325 port->aggregator->partner_oper_aggregator_key = port->partner_oper.key;
1326 port->aggregator->receive_state = 1;
1327 port->aggregator->transmit_state = 1;
1328 port->aggregator->lag_ports = port;
1329 port->aggregator->num_of_ports++;
1330
1331 /* mark this port as selected */
1332 port->sm_vars |= AD_PORT_SELECTED;
1333
1334 netdev_dbg(bond->dev, "Port %d joined LAG %d(new LAG)\n",
1335 port->actor_port_number,
1336 port->aggregator->aggregator_identifier);
1337 } else {
1338 netdev_err(bond->dev, "Port %d (on %s) did not find a suitable aggregator\n",
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Uh, oh. We totally failed to find a good port->aggretator.
1339 port->actor_port_number, port->slave->dev->name);
1340 }
1341 }
1342 /* if all aggregator's ports are READY_N = TRUE, set ready=TRUE
1343 * in all aggregator's ports, else set readyúLSE in all
1344 * aggregator's ports
1345 */
1346 __set_agg_ports_ready(port->aggregator,
^^^^^^^^^^^^^^^^
We dereference the NULL inside the call to __set_agg_ports_ready().
1347 __agg_ports_are_ready(port->aggregator)); 1348
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html