Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1246956

Re: [PATCH] staging: octeon: fixed few coding style warnings

Show all headers | View raw

On 10/14/2015 07:06 AM, Sakshi Bansal wrote:
> Fixed allignment issues and line over 80 characters

Use spell checking on 'allignment'

But that is not the main problem with the patch...


You are changing things other than white space and comment formatting, 
can you tell us on which platforms the patch was tested to verify that 
you didn't break anything?

>
> Signed-off-by: Sakshi Bansal <sakshi.april5@gmail.com>


NAK.

[...]
> diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
> index fd9b3d8..590a6cb 100644
> --- a/drivers/staging/octeon/ethernet-mdio.c
> +++ b/drivers/staging/octeon/ethernet-mdio.c
> @@ -180,7 +180,7 @@ int cvm_oct_phy_setup_device(struct net_device *dev)
>   	priv->phydev = of_phy_connect(dev, phy_node, cvm_oct_adjust_link, 0,
>   				      PHY_INTERFACE_MODE_GMII);
>
> -	if (priv->phydev == NULL)
> +	if (!priv->phydev)


Not a coding style change.  There is no WARNING generated for this case.



>   		return -ENODEV;
>
>   	priv->last_link = 0;
> diff --git a/drivers/staging/octeon/ethernet-mem.c b/drivers/staging/octeon/ethernet-mem.c
> index 5a5cdb3..d6172e4 100644
> --- a/drivers/staging/octeon/ethernet-mem.c
> +++ b/drivers/staging/octeon/ethernet-mem.c
> @@ -34,7 +34,7 @@ static int cvm_oct_fill_hw_skbuff(int pool, int size, int elements)
>   	while (freed) {
>   		struct sk_buff *skb = dev_alloc_skb(size + 256);
>
> -		if (unlikely(skb == NULL))
> +		if (unlikely(!skb))

Same

>   			break;
>   		skb_reserve(skb, 256 - (((unsigned long)skb->data) & 0x7f));
>   		*(struct sk_buff **)(skb->data - sizeof(void *)) = skb;
> @@ -98,7 +98,7 @@ static int cvm_oct_fill_hw_memory(int pool, int size, int elements)
>   		 * just before the block.
>   		 */
>   		memory = kmalloc(size + 256, GFP_ATOMIC);
> -		if (unlikely(memory == NULL)) {
> +		if (unlikely(!memory)) {

Same

>   			pr_warn("Unable to allocate %u bytes for FPA pool %d\n",
>   				elements * size, pool);
>   			break;
> diff --git a/drivers/staging/octeon/ethernet-rgmii.c b/drivers/staging/octeon/ethernet-rgmii.c
> index 51dcb61..3d7513c 100644
> --- a/drivers/staging/octeon/ethernet-rgmii.c
> +++ b/drivers/staging/octeon/ethernet-rgmii.c
> @@ -68,7 +68,7 @@ static void cvm_oct_rgmii_poll(struct net_device *dev)
>   	struct octeon_ethernet *priv = netdev_priv(dev);
>   	unsigned long flags = 0;
>   	cvmx_helper_link_info_t link_info;
> -	int use_global_register_lock = (priv->phydev == NULL);
> +	int use_global_register_lock = (!priv->phydev);

Same.

I could go on, but I think you see the pattern here.

Your changelog says you are fixing warnings, but none of these are 
warning fixes.

In fact it is perfectly acceptable to compare a pointer to NULL.  It is 
a common idiom in the kernel.  The original author of the code thought 
it was more clear this way, and you are causing code churn for no reason.

Try to run this command on the kernel sources:
$ git grep -e '== NULL' | wc -l
21488

I would suggest that you convince people that the other 21,000 cases of 
comparison to NULL need changing before you do it to this driver.

David Daney

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] staging: octeon: fixed few coding style warnings Sakshi Bansal <sakshi.april5@gmail.com> - 2015-10-14 16:10 +0200
  Re: [PATCH] staging: octeon: fixed few coding style warnings David Daney <ddaney@caviumnetworks.com> - 2015-10-14 18:10 +0200
    Re: [PATCH] staging: octeon: fixed few coding style warnings Arnd Bergmann <arnd@arndb.de> - 2015-10-14 23:00 +0200
      Re: [PATCH] staging: octeon: fixed few coding style warnings David Daney <ddaney@caviumnetworks.com> - 2015-10-14 23:10 +0200
        Re: [PATCH] staging: octeon: fixed few coding style warnings Arnd Bergmann <arnd@arndb.de> - 2015-10-14 23:40 +0200
    Re: [PATCH] staging: octeon: fixed few coding style warnings Dan Carpenter <dan.carpenter@oracle.com> - 2015-10-15 15:50 +0200
      Re: [PATCH] staging: octeon: fixed few coding style warnings Dan Carpenter <dan.carpenter@oracle.com> - 2015-10-15 20:20 +0200
  Re: [PATCH] staging: octeon: fixed few coding style warnings Aaro Koskinen <aaro.koskinen@iki.fi> - 2015-10-14 21:40 +0200

csiph-web