ehea: ethtool interface

drivers/net/ehea/ehea_ethtool.c +11 -0

+	struct ehea_port *port = netdev_priv(dev);
+	struct ehea_adapter *adapter = port->adapter;
+	struct hcp_query_ehea_port_cb_4 *cb4 = NULL;
+
+	cb4 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
+	if(!cb4) {
+		ehea_error("no mem for cb4");
+		goto get_settings_exit;
+	}
+
+	hret = ehea_h_query_ehea_port(adapter->handle,

+static u32 netdev_get_msglevel(struct net_device *dev)

drivers/net/ehea/ehea_ethtool.c +9 -0

+	cb6 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
+	if(!cb6) {
+		ehea_error("no mem for cb6");
+		goto stats_exit;
+	}
+
+	hret = ehea_h_query_ehea_port(adapter->handle,
+				      port->logical_port_id,
+				      H_PORT_CB6,

drivers/net/ehea/ehea_ethtool.c +8 -0

+	.set_tx_csum = ethtool_op_set_tx_csum,
+	.get_sg = ethtool_op_get_sg,
+	.set_sg = ethtool_op_set_sg,
+	.get_tso = ethtool_op_get_tso,
+	.set_tso = ethtool_op_set_tso,
+	.get_strings = netdev_get_strings,
+	.get_stats_count = netdev_get_stats_count,
+	.get_ethtool_stats = netdev_get_ethtool_stats,
 
 

Review

Quoted 1 line

(quoted diff) +1 -0

+	u64 hret = H_HARDWARE;

Useless assignment here and everywhere.

drivers/net/ehea/ehea_ethtool.c line 116

Quoted 5 lines

drivers/net/ehea/ehea_ethtool.c +5 -0

+static void netdev_get_drvinfo(struct net_device *dev,
+			       struct ethtool_drvinfo *info)
+{
+	strncpy(info->driver, DRV_NAME, sizeof(info->driver) - 1);
+	strncpy(info->version, DRV_VERSION, sizeof(info->version) - 1);

Use strlcpy() to not forget -1 accidently.

Hi Alexey,

first of all thanks a lot for the extensive review.

Alexey Dobriyan wrote:

Quoted 4 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	u64 hret = H_HARDWARE;

Useless assignment here and everywhere.

Initializing returncodes to errorstate is a cheap way to prevent accidentally returning (uninitalized) success returncodes which can lead to catastrophic misbehaviour.

Quoted 9 lines · On Fri, 18 Aug 2006 17:41:26 +0200 Thomas Klein <[email protected]> wrote:

Hi Alexey,

first of all thanks a lot for the extensive review.

Quoted 3 lines · Alexey Dobriyan

Review

Quoted 1 line

(quoted diff) +1 -0

+	u64 hret = H_HARDWARE;

Useless assignment here and everywhere.

Quoted 3 lines

Initializing returncodes to errorstate is a cheap way to prevent accidentally returning (uninitalized) success returncodes which can lead to catastrophic misbehaviour.

That is old thinking. Current compilers do live/dead analysis and tell you about this at compile time which is better than relying on default behavior at runtime.

Hi Alexey,

first of all thanks a lot for the extensive review.

Alexey Dobriyan wrote:

Quoted 4 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	u64 hret = H_HARDWARE;

Useless assignment here and everywhere.

Initializing returncodes to errorstate is a cheap way to prevent accidentally returning (uninitalized) success returncodes which can lead to catastrophic misbehaviour.

If you try to return an uninitialized value the compiler will warn you, you'll then look at the code and realise you missed a case, you might save yourself a bug.

Quoted 4 lines · Michael Ellerman

If you try to return an uninitialized value the compiler will warn you, you'll then look at the code and realise you missed a case, you might save yourself a bug.

You *should* look at the code :)

So should we be reporting these as bugs?

A lot of these have started appearing recently, which I think is due to GCC becoming more vocal. Unfortunately many of them are false positives caused by GCC not seeming to grok that this is ok:

void foo(int *x) { *x = 1; }
...
int x;
foo(&x);
return x;

fs/bio.c:169: warning: 'idx' may be used uninitialized in this function fs/eventpoll.c:500: warning: 'fd' may be used uninitialized in this function fs/isofs/namei.c:162: warning: 'offset' may be used uninitialized in this function fs/isofs/namei.c:162: warning: 'block' may be used uninitialized in this function fs/nfsd/nfsctl.c:292: warning: 'maxsize' may be used uninitialized in this function fs/udf/balloc.c:751: warning: 'goal_eloc.logicalBlockNum' may be used uninitialized in this function fs/udf/super.c:1358: warning: 'ino.partitionReferenceNum' may be used uninitialized in this function fs/xfs/xfs_alloc_btree.c:611: warning: 'nkey.ar_startblock' may be used uninitialized in this function fs/xfs/xfs_alloc_btree.c:611: warning: 'nkey.ar_blockcount' may be used uninitialized in this function fs/xfs/xfs_bmap.c:2498: warning: 'rtx' is used uninitialized in this function fs/xfs/xfs_bmap_btree.c:753: warning: 'nkey.br_startoff' may be used uninitialized in this function fs/xfs/xfs_da_btree.c:151: warning: 'action' may be used uninitialized in this function fs/xfs/xfs_dir.c:363: warning: 'totallen' may be used uninitialized in this function fs/xfs/xfs_dir.c:363: warning: 'count' may be used uninitialized in this function fs/xfs/xfs_ialloc_btree.c:545: warning: 'nkey.ir_startino' may be used uninitialized in this function fs/xfs/xfs_inode.c:1958: warning: 'last_dip' may be used uninitialized in this function fs/xfs/xfs_inode.c:1960: warning: 'last_offset' may be used uninitialized in this function fs/xfs/xfs_log.c:1749: warning: 'iclog' may be used uninitialized in this function fs/xfs/xfs_log_recover.c:523: warning: 'first_blk' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.qbytes' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.uid' may be used uninitialized in this function ipc/msg.c:338: warning: 'setbuf.gid' may be used uninitialized in this function

ipc/msg.c:338: warning: 'setbuf.mode' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.uid' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.gid' may be used uninitialized in this function ipc/sem.c:810: warning: 'setbuf.mode' may be used uninitialized in this function drivers/md/dm-table.c:431: warning: 'dev' may be used uninitialized in this function drivers/md/dm-ioctl.c:1388: warning: 'param' may be used uninitialized in this function net/sched/sch_cbq.c:409: warning: 'ret' may be used uninitialized in this function lib/zlib_inflate/inftrees.c:121: warning: 'r.base' may be used uninitialized in this function

drivers/net/ehea/ehea_ethtool.c line 116

Quoted 5 lines · Jan-Bernd Themann

drivers/net/ehea/ehea_ethtool.c +3 -0

+{
+    strncpy(info->driver, DRV_NAME, sizeof(info->driver) - 1);
+    strncpy(info->version, DRV_VERSION, sizeof(info->version) - 1);

Use strlcpy() to not forget -1 accidently.

Quoted 2 lines

+static u32 netdev_get_msglevel(struct net_device *dev)

                       ^^^^^^^^

drivers/net/ehea/ehea_ethtool.c line 122

Quoted 3 lines

drivers/net/ehea/ehea_ethtool.c +3 -0

+{
+    struct ehea_port *port = netdev_priv(dev);
+    return port->msg_enable;

                       ^^^^^^

Something is mis-named here.

drivers/net/ehea/ehea_ethtool.c line 241

Quoted 7 lines

drivers/net/ehea/ehea_ethtool.c +7 -0

+struct ethtool_ops ehea_ethtool_ops = {
+    .get_settings = netdev_get_settings,
+    .get_drvinfo = netdev_get_drvinfo,
+    .get_msglevel = netdev_get_msglevel,
+    .set_msglevel = netdev_set_msglevel,
+        .get_link = ethtool_op_get_link,
+        .get_tx_csum = ethtool_op_get_tx_csum,

Whitespace breakage.

drivers/net/ehea/ehea_ethtool.c line 258

Quoted 9 lines

drivers/net/ehea/ehea_ethtool.c +9 -0

+    .set_settings = NULL,
+    .nway_reset = NULL,
+    .get_pauseparam = NULL,
+    .set_pauseparam = NULL,
+    .get_rx_csum = NULL,
+    .set_rx_csum = NULL,
+    .phys_id = NULL,
+    .self_test = NULL,
+    .self_test_count = NULL

If you don't use them, don't mention them at all. Compiler will DTRT.