netdev - Re: [PATCH V2 1/9] net/mlx5: Mellanox Connect-IB, core driver part 1/3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201307041626.45125.jackm@dev.mellanox.co.il>
Date:	Thu, 4 Jul 2013 16:26:44 +0300
From:	Jack Morgenstein <jackm@....mellanox.co.il>
To:	Joe Perches <joe@...ches.com>
Cc:	Or Gerlitz <ogerlitz@...lanox.com>, roland@...nel.org,
	linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
	davem@...emloft.net, eli@....mellanox.co.il, moshel@...lanox.com,
	Eli Cohen <eli@...lanox.com>
Subject: Re: [PATCH V2 1/9] net/mlx5: Mellanox Connect-IB, core driver part 1/3

On Wednesday 03 July 2013 23:29, Joe Perches wrote:
> On Wed, 2013-07-03 at 20:13 +0300, Or Gerlitz wrote:
> > From: Eli Cohen <eli@...lanox.com>
> 
> trivial comments:
> 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> []
> > +static const char *deliv_status_to_str(u8 status)
> > +{
> > +	switch (status) {
> > +	case MLX5_CMD_DELIVERY_STAT_OK:
> > +		return "no errors";
> []
> > +	default:
> > +		return "unknown status code\n";
> > +	}
> > +}
> Likely unnecessary newline for default case
All the cases here have newlines between them, to enhance readability.
(not just the default). If you feel strongly about this, I'll do the change
for V3.

> > +static struct mlx5_cmd_mailbox *alloc_cmd_box(struct mlx5_core_dev *dev,
> > +					      gfp_t flags)
> > +{
> > +	struct mlx5_cmd_mailbox *mailbox;
> > +
> > +	mailbox = kmalloc(sizeof(*mailbox), flags);
> > +	if (!mailbox) {
> > +		mlx5_core_dbg(dev, "failed allocation\n");
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> 
> unnecessary OOM message.

Will remove these kmalloc failure printouts in V3
> 
> > +static void set_wqname(struct mlx5_core_dev *dev)
> > +{
> > +	struct mlx5_cmd *cmd = &dev->cmd;
> > +
> > +	strcpy(cmd->wq_name, "mlx5_cmd_");
> > +	strcat(cmd->wq_name, dev_name(&dev->pdev->dev));
> 
> More likely snprintf might be better.
> 
> 	snprintf(cmd->wq_name, sizeof(cmd->wq_name), "mlx5_cmd_%s",
> 		 dev_name(&dev->pdev->dev));
> 
Will change this for V3
> How big is wq_name?
32 bytes. Should be enough to hold dev names. However, to be on the safe side
I'll do the change.

-Jack
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html