Re: [PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-07 Thread arvind Yadav 

As per your concern, I have change commit message. Submit other patch.

Thanks,
Arvind Yadav

On Thursday 07 July 2016 01:40 PM, Arnd Bergmann wrote:

On Thursday, July 7, 2016 12:47:43 AM CEST Arvind Yadav wrote:

 Most users of IS_ERR_VALUE() in the kernel are wrong, as they
 pass an 'int' into a function that takes an 'unsigned long'
 argument. This happens to work because the type is sign-extended
 on 64-bit architectures before it gets converted into an
 unsigned type.

 However, anything that passes an 'unsigned short' or 'unsigned int'
 argument into IS_ERR_VALUE() is guaranteed to be broken, as are
 8-bit integers and types that are wider than 'unsigned long'.

 Andrzej Hajda has already fixed a lot of the worst abusers that
 were causing actual bugs, but it would be nice to prevent any
 users that are not passing 'unsigned long' arguments.

 This patch changes all users of IS_ERR_VALUE() that I could find
 on 32-bit ARM randconfig builds and x86 allmodconfig. For the
 moment, this doesn't change the definition of IS_ERR_VALUE()
 because there are probably still architecture specific users
 elsewhere.

 Almost all the warnings I got are for files that are better off
 using 'if (err)' or 'if (err < 0)'.
 The only legitimate user I could find that we get a warning for
 is the (32-bit only) freescale gigabit ethernet driver.

 I was using this definition for testing:

  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
   unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

 which ends up making all 16-bit or wider types work correctly with
 the most plausible interpretation of what IS_ERR_VALUE() was supposed
 to return according to its users, but also causes a compile-time
 warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 

The above is my changelog text for commit 287980e49f, but it's not my patch.

Your changes look fine, but reusing my description there seems wrong.
Can you add a new description and just point to my commit instead?

Arnd


  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx)

Re: [PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-07 Thread arvind Yadav 

As per your concern, I have change commit message. Submit other patch.

Thanks,
Arvind Yadav

On Thursday 07 July 2016 01:40 PM, Arnd Bergmann wrote:

On Thursday, July 7, 2016 12:47:43 AM CEST Arvind Yadav wrote:

 Most users of IS_ERR_VALUE() in the kernel are wrong, as they
 pass an 'int' into a function that takes an 'unsigned long'
 argument. This happens to work because the type is sign-extended
 on 64-bit architectures before it gets converted into an
 unsigned type.

 However, anything that passes an 'unsigned short' or 'unsigned int'
 argument into IS_ERR_VALUE() is guaranteed to be broken, as are
 8-bit integers and types that are wider than 'unsigned long'.

 Andrzej Hajda has already fixed a lot of the worst abusers that
 were causing actual bugs, but it would be nice to prevent any
 users that are not passing 'unsigned long' arguments.

 This patch changes all users of IS_ERR_VALUE() that I could find
 on 32-bit ARM randconfig builds and x86 allmodconfig. For the
 moment, this doesn't change the definition of IS_ERR_VALUE()
 because there are probably still architecture specific users
 elsewhere.

 Almost all the warnings I got are for files that are better off
 using 'if (err)' or 'if (err < 0)'.
 The only legitimate user I could find that we get a warning for
 is the (32-bit only) freescale gigabit ethernet driver.

 I was using this definition for testing:

  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
   unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

 which ends up making all 16-bit or wider types work correctly with
 the most plausible interpretation of what IS_ERR_VALUE() was supposed
 to return according to its users, but also causes a compile-time
 warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 

The above is my changelog text for commit 287980e49f, but it's not my patch.

Your changes look fine, but reusing my description there seems wrong.
Can you add a new description and just point to my commit instead?

Arnd


  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +

[PATCH] Remove lots of IS_ERR_VALUE abuses and compilation warning.

2016-07-07 Thread Arvind Yadav 
Passing value in IS_ERR_VALUE() is wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

Passing an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
'unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
worst abusers.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

[PATCH] Remove lots of IS_ERR_VALUE abuses and compilation warning.

2016-07-07 Thread Arvind Yadav 
Passing value in IS_ERR_VALUE() is wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

Passing an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Any user will get compilation warning for that do not pass an
'unsigned long' argument.

Commit '287980e49f; - This change is alreday fixes lots of other
worst abusers.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

Re: [PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-07 Thread Arnd Bergmann 
On Thursday, July 7, 2016 12:47:43 AM CEST Arvind Yadav wrote:
> Most users of IS_ERR_VALUE() in the kernel are wrong, as they
> pass an 'int' into a function that takes an 'unsigned long'
> argument. This happens to work because the type is sign-extended
> on 64-bit architectures before it gets converted into an
> unsigned type.
> 
> However, anything that passes an 'unsigned short' or 'unsigned int'
> argument into IS_ERR_VALUE() is guaranteed to be broken, as are
> 8-bit integers and types that are wider than 'unsigned long'.
> 
> Andrzej Hajda has already fixed a lot of the worst abusers that
> were causing actual bugs, but it would be nice to prevent any
> users that are not passing 'unsigned long' arguments.
> 
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.
> 
> Almost all the warnings I got are for files that are better off
> using 'if (err)' or 'if (err < 0)'.
> The only legitimate user I could find that we get a warning for
> is the (32-bit only) freescale gigabit ethernet driver.
> 
> I was using this definition for testing:
> 
>  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
>   unlikely((unsigned long long)(x) >= (unsigned long 
> long)(typeof(x))-MAX_ERRNO))
> 
> which ends up making all 16-bit or wider types work correctly with
> the most plausible interpretation of what IS_ERR_VALUE() was supposed
> to return according to its users, but also causes a compile-time
> warning for any users that do not pass an 'unsigned long' argument.
> 
> Signed-off-by: Arvind Yadav 

The above is my changelog text for commit 287980e49f, but it's not my patch.

Your changes look fine, but reusing my description there seems wrong.
Can you add a new description and just point to my commit instead?

Arnd

>  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
> b/drivers/net/ethernet/freescale/ucc_geth.c
> index 5bf1ade..c1ead2c 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
> *ugeth,
>   else {
>   init_enet_offset =
>   qe_muram_alloc(thread_size, thread_alignment);
> - if (IS_ERR_VALUE(init_enet_offset)) {
> + if (init_enet_offset < 0) {
>   if (netif_msg_ifup(ugeth))
>   pr_err("Can not allocate DPRAM 
> memory\n");
>   qe_put_snum((u8) snum);
> @@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
> *ugeth)
>   ugeth->tx_bd_ring_offset[j] =
>   qe_muram_alloc(length,
>  UCC_GETH_TX_BD_RING_ALIGNMENT);
> - if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
> + if (!ugeth->tx_bd_ring_offset[j])
>   ugeth->p_tx_bd_ring[j] =
>   (u8 __iomem *) qe_muram_addr(ugeth->
>tx_bd_ring_offset[j]);
> @@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
> *ugeth)
>   ugeth->rx_bd_ring_offset[j] =
>   qe_muram_alloc(length,
>  UCC_GETH_RX_BD_RING_ALIGNMENT);
> - if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
> + if (!ugeth->rx_bd_ring_offset[j])
>   ugeth->p_rx_bd_ring[j] =
>   (u8 __iomem *) qe_muram_addr(ugeth->
>rx_bd_ring_offset[j]);
> @@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
>   ugeth->tx_glbl_pram_offset =
>   qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
>  UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
> - if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
> + if (ugeth->tx_glbl_pram_offset < 0) {
>   if (netif_msg_ifup(ugeth))
>   pr_err("Can not allocate DPRAM memory for 
> p_tx_glbl_pram\n");
>   return -ENOMEM;
> @@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
>  sizeof(struct ucc_geth_thread_data_tx) +
>  32 * (numThreadsTxNumerical == 1),
>

Re: [PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-07 Thread Arnd Bergmann 
On Thursday, July 7, 2016 12:47:43 AM CEST Arvind Yadav wrote:
> Most users of IS_ERR_VALUE() in the kernel are wrong, as they
> pass an 'int' into a function that takes an 'unsigned long'
> argument. This happens to work because the type is sign-extended
> on 64-bit architectures before it gets converted into an
> unsigned type.
> 
> However, anything that passes an 'unsigned short' or 'unsigned int'
> argument into IS_ERR_VALUE() is guaranteed to be broken, as are
> 8-bit integers and types that are wider than 'unsigned long'.
> 
> Andrzej Hajda has already fixed a lot of the worst abusers that
> were causing actual bugs, but it would be nice to prevent any
> users that are not passing 'unsigned long' arguments.
> 
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.
> 
> Almost all the warnings I got are for files that are better off
> using 'if (err)' or 'if (err < 0)'.
> The only legitimate user I could find that we get a warning for
> is the (32-bit only) freescale gigabit ethernet driver.
> 
> I was using this definition for testing:
> 
>  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
>   unlikely((unsigned long long)(x) >= (unsigned long 
> long)(typeof(x))-MAX_ERRNO))
> 
> which ends up making all 16-bit or wider types work correctly with
> the most plausible interpretation of what IS_ERR_VALUE() was supposed
> to return according to its users, but also causes a compile-time
> warning for any users that do not pass an 'unsigned long' argument.
> 
> Signed-off-by: Arvind Yadav 

The above is my changelog text for commit 287980e49f, but it's not my patch.

Your changes look fine, but reusing my description there seems wrong.
Can you add a new description and just point to my commit instead?

Arnd

>  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
> b/drivers/net/ethernet/freescale/ucc_geth.c
> index 5bf1ade..c1ead2c 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
> *ugeth,
>   else {
>   init_enet_offset =
>   qe_muram_alloc(thread_size, thread_alignment);
> - if (IS_ERR_VALUE(init_enet_offset)) {
> + if (init_enet_offset < 0) {
>   if (netif_msg_ifup(ugeth))
>   pr_err("Can not allocate DPRAM 
> memory\n");
>   qe_put_snum((u8) snum);
> @@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
> *ugeth)
>   ugeth->tx_bd_ring_offset[j] =
>   qe_muram_alloc(length,
>  UCC_GETH_TX_BD_RING_ALIGNMENT);
> - if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
> + if (!ugeth->tx_bd_ring_offset[j])
>   ugeth->p_tx_bd_ring[j] =
>   (u8 __iomem *) qe_muram_addr(ugeth->
>tx_bd_ring_offset[j]);
> @@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
> *ugeth)
>   ugeth->rx_bd_ring_offset[j] =
>   qe_muram_alloc(length,
>  UCC_GETH_RX_BD_RING_ALIGNMENT);
> - if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
> + if (!ugeth->rx_bd_ring_offset[j])
>   ugeth->p_rx_bd_ring[j] =
>   (u8 __iomem *) qe_muram_addr(ugeth->
>rx_bd_ring_offset[j]);
> @@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
>   ugeth->tx_glbl_pram_offset =
>   qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
>  UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
> - if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
> + if (ugeth->tx_glbl_pram_offset < 0) {
>   if (netif_msg_ifup(ugeth))
>   pr_err("Can not allocate DPRAM memory for 
> p_tx_glbl_pram\n");
>   return -ENOMEM;
> @@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
>  sizeof(struct ucc_geth_thread_data_tx) +
>  32 * (numThreadsTxNumerical == 1),
>

[PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-06 Thread Arvind Yadav 
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale QE UCC Fast API.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
  unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

[PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-06 Thread Arvind Yadav 
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale QE UCC Fast API.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
  unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/soc/fsl/qe/ucc_fast.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index a768931..7cc783c 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -268,7 +268,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
@@ -281,7 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct 
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
   UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
   UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
-   if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+   if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
-- 
1.9.1

[PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-06 Thread Arvind Yadav 
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale gigabit ethernet driver.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
  unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_thread_data_tx\n");
return -ENOMEM;
@@ -2568,7 +2568,7 @@ static int ucc_geth_startup(struct ucc_geth_private

[PATCH] Remove lots of IS_ERR_VALUE abuses.

2016-07-06 Thread Arvind Yadav 
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale gigabit ethernet driver.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
  unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/freescale/ucc_geth.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
b/drivers/net/ethernet/freescale/ucc_geth.c
index 5bf1ade..c1ead2c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -289,7 +289,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
*ugeth,
else {
init_enet_offset =
qe_muram_alloc(thread_size, thread_alignment);
-   if (IS_ERR_VALUE(init_enet_offset)) {
+   if (init_enet_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM 
memory\n");
qe_put_snum((u8) snum);
@@ -2234,7 +2234,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
*ugeth)
ugeth->tx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_TX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
+   if (!ugeth->tx_bd_ring_offset[j])
ugeth->p_tx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 tx_bd_ring_offset[j]);
@@ -2311,7 +2311,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
*ugeth)
ugeth->rx_bd_ring_offset[j] =
qe_muram_alloc(length,
   UCC_GETH_RX_BD_RING_ALIGNMENT);
-   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
+   if (!ugeth->rx_bd_ring_offset[j])
ugeth->p_rx_bd_ring[j] =
(u8 __iomem *) qe_muram_addr(ugeth->
 rx_bd_ring_offset[j]);
@@ -2521,7 +2521,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
ugeth->tx_glbl_pram_offset =
qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
   UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
+   if (ugeth->tx_glbl_pram_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_tx_glbl_pram\n");
return -ENOMEM;
@@ -2541,7 +2541,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)
   sizeof(struct ucc_geth_thread_data_tx) +
   32 * (numThreadsTxNumerical == 1),
   UCC_GETH_THREAD_DATA_ALIGNMENT);
-   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
+   if (ugeth->thread_dat_tx_offset < 0) {
if (netif_msg_ifup(ugeth))
pr_err("Can not allocate DPRAM memory for 
p_thread_data_tx\n");
return -ENOMEM;
@@ -2568,7 +2568,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
*ugeth)

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Linus Torvalds 
On Fri, May 27, 2016 at 2:23 PM, Arnd Bergmann  wrote:
>
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.

Patch applied with the fixups from Al Viro edited in.

I also ended up removing a few other users (due to the vm_brk()
interface), and then made IS_ERR_VALUE() do the "void *" cast so that
integer use of a non-pointer size should now complain.

It works for me and has no new warnings in my allmodconfig build, and
with your ARM work that is presumably clean too. But other
architectures may see new warnings.

People who got affected by this should check their subsystem code for
the changes.

  Linus

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Linus Torvalds 
On Fri, May 27, 2016 at 2:23 PM, Arnd Bergmann  wrote:
>
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.

Patch applied with the fixups from Al Viro edited in.

I also ended up removing a few other users (due to the vm_brk()
interface), and then made IS_ERR_VALUE() do the "void *" cast so that
integer use of a non-pointer size should now complain.

It works for me and has no new warnings in my allmodconfig build, and
with your ARM work that is presumably clean too. But other
architectures may see new warnings.

People who got affected by this should check their subsystem code for
the changes.

  Linus

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Al Viro 
On Fri, May 27, 2016 at 11:23:25PM +0200, Arnd Bergmann wrote:

> @@ -837,7 +837,7 @@ static int load_flat_shared_library(int id, struct 
> lib_info *libs)
>  
>   res = prepare_binprm();
>  
> - if (!IS_ERR_VALUE(res))
> + if (res >= 0)

if (res == 0), please - prepare_binprm() returns 0 or -E...

> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -521,7 +521,7 @@ static int p9_check_errors(struct p9_client *c, struct 
> p9_req_t *req)
>   if (p9_is_proto_dotu(c))
>   err = -ecode;
>  
> - if (!err || !IS_ERR_VALUE(err)) {
> + if (!err || !IS_ERR_VALUE((unsigned long)err)) {

Not really - it's actually
if (p9_is_proto_dotu(c) && ecode < 512)
err = -ecode;

if (!err) {
...
> @@ -608,7 +608,7 @@ static int p9_check_zc_errors(struct p9_client *c, struct 
> p9_req_t *req,
>   if (p9_is_proto_dotu(c))
>   err = -ecode;
>  
> - if (!err || !IS_ERR_VALUE(err)) {
> + if (!err || !IS_ERR_VALUE((unsigned long)err)) {

Ditto.

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Al Viro 
On Fri, May 27, 2016 at 11:23:25PM +0200, Arnd Bergmann wrote:

> @@ -837,7 +837,7 @@ static int load_flat_shared_library(int id, struct 
> lib_info *libs)
>  
>   res = prepare_binprm();
>  
> - if (!IS_ERR_VALUE(res))
> + if (res >= 0)

if (res == 0), please - prepare_binprm() returns 0 or -E...

> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -521,7 +521,7 @@ static int p9_check_errors(struct p9_client *c, struct 
> p9_req_t *req)
>   if (p9_is_proto_dotu(c))
>   err = -ecode;
>  
> - if (!err || !IS_ERR_VALUE(err)) {
> + if (!err || !IS_ERR_VALUE((unsigned long)err)) {

Not really - it's actually
if (p9_is_proto_dotu(c) && ecode < 512)
err = -ecode;

if (!err) {
...
> @@ -608,7 +608,7 @@ static int p9_check_zc_errors(struct p9_client *c, struct 
> p9_req_t *req,
>   if (p9_is_proto_dotu(c))
>   err = -ecode;
>  
> - if (!err || !IS_ERR_VALUE(err)) {
> + if (!err || !IS_ERR_VALUE((unsigned long)err)) {

Ditto.

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Srinivas Kandagatla 



On 27/05/16 22:23, Arnd Bergmann wrote:

  drivers/acpi/acpi_dbg.c  | 22 +++---
  drivers/ata/sata_highbank.c  |  2 +-
  drivers/clk/tegra/clk-tegra210.c |  2 +-
  drivers/cpufreq/omap-cpufreq.c   |  2 +-
  drivers/crypto/caam/ctrl.c   |  2 +-
  drivers/dma/sun4i-dma.c  | 16 
  drivers/gpio/gpio-xlp.c  |  2 +-
  drivers/gpu/drm/sti/sti_vtg.c|  4 ++--
  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  2 +-
  drivers/gpu/host1x/hw/intr_hw.c  |  2 +-
  drivers/iommu/arm-smmu-v3.c  | 18 +-

...

  drivers/mmc/host/sdhci-of-at91.c |  2 +-
  drivers/mmc/host/sdhci.c |  4 ++--
  drivers/net/ethernet/freescale/fman/fman.c   |  2 +-
  drivers/net/ethernet/freescale/fman/fman_muram.c |  4 ++--
  drivers/net/ethernet/freescale/fman/fman_muram.h |  4 ++--
  drivers/net/wireless/ti/wlcore/spi.c |  4 ++--
  drivers/nvmem/core.c | 22 +++---


For nvmem part,

Acked-by: Srinivas Kandagatla 


  drivers/tty/serial/amba-pl011.c  |  2 +-
  drivers/tty/serial/sprd_serial.c |  2 +-
  drivers/video/fbdev/da8xx-fb.c   |  4 ++--
  fs/afs/write.c   |  4 
  fs/binfmt_flat.c |  6 +++---
  fs/gfs2/dir.c| 15 +--
  kernel/pid.c |  2 +-
  net/9p/client.c  |  4 ++--
  sound/soc/qcom/lpass-platform.c  |  4 ++--

There is already a patch for this in Mark Brown's sound tree,

https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/commit/?h=for-linus=cef794f76485f4a4e6affd851ae9f9d0eb4287a5

Thanks,
srini

  38 files changed, 100 insertions(+), 101 deletions(-)

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Srinivas Kandagatla 



On 27/05/16 22:23, Arnd Bergmann wrote:

  drivers/acpi/acpi_dbg.c  | 22 +++---
  drivers/ata/sata_highbank.c  |  2 +-
  drivers/clk/tegra/clk-tegra210.c |  2 +-
  drivers/cpufreq/omap-cpufreq.c   |  2 +-
  drivers/crypto/caam/ctrl.c   |  2 +-
  drivers/dma/sun4i-dma.c  | 16 
  drivers/gpio/gpio-xlp.c  |  2 +-
  drivers/gpu/drm/sti/sti_vtg.c|  4 ++--
  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  2 +-
  drivers/gpu/host1x/hw/intr_hw.c  |  2 +-
  drivers/iommu/arm-smmu-v3.c  | 18 +-

...

  drivers/mmc/host/sdhci-of-at91.c |  2 +-
  drivers/mmc/host/sdhci.c |  4 ++--
  drivers/net/ethernet/freescale/fman/fman.c   |  2 +-
  drivers/net/ethernet/freescale/fman/fman_muram.c |  4 ++--
  drivers/net/ethernet/freescale/fman/fman_muram.h |  4 ++--
  drivers/net/wireless/ti/wlcore/spi.c |  4 ++--
  drivers/nvmem/core.c | 22 +++---


For nvmem part,

Acked-by: Srinivas Kandagatla 


  drivers/tty/serial/amba-pl011.c  |  2 +-
  drivers/tty/serial/sprd_serial.c |  2 +-
  drivers/video/fbdev/da8xx-fb.c   |  4 ++--
  fs/afs/write.c   |  4 
  fs/binfmt_flat.c |  6 +++---
  fs/gfs2/dir.c| 15 +--
  kernel/pid.c |  2 +-
  net/9p/client.c  |  4 ++--
  sound/soc/qcom/lpass-platform.c  |  4 ++--

There is already a patch for this in Mark Brown's sound tree,

https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/commit/?h=for-linus=cef794f76485f4a4e6affd851ae9f9d0eb4287a5

Thanks,
srini

  38 files changed, 100 insertions(+), 101 deletions(-)

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Linus Torvalds 
On Fri, May 27, 2016 at 2:46 PM, Andrew Morton
 wrote:
>
> So you do plan to add some sort of typechecking into IS_ERR_VALUE()?

The easiest way to do it is to just turn the (x) into (unsigned
long)(void *)(x), which then complains about casting an integer to a
pointer if the integer has the wrong size.

But if we get rid of the bogus cases, there's just a few left, and we
should probably just rename the whole thing (the initial double
underscore). It really isn't something normal people should use.

  Linus

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Linus Torvalds 
On Fri, May 27, 2016 at 2:46 PM, Andrew Morton
 wrote:
>
> So you do plan to add some sort of typechecking into IS_ERR_VALUE()?

The easiest way to do it is to just turn the (x) into (unsigned
long)(void *)(x), which then complains about casting an integer to a
pointer if the integer has the wrong size.

But if we get rid of the bogus cases, there's just a few left, and we
should probably just rename the whole thing (the initial double
underscore). It really isn't something normal people should use.

  Linus

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Andrew Morton 
On Fri, 27 May 2016 23:23:25 +0200 Arnd Bergmann  wrote:

> Most users of IS_ERR_VALUE() in the kernel are wrong, as they
> pass an 'int' into a function that takes an 'unsigned long'
> argument. This happens to work because the type is sign-extended
> on 64-bit architectures before it gets converted into an
> unsigned type.
> 
> However, anything that passes an 'unsigned short' or 'unsigned int'
> argument into IS_ERR_VALUE() is guaranteed to be broken, as are
> 8-bit integers and types that are wider than 'unsigned long'.
> 
> Andrzej Hajda has already fixed a lot of the worst abusers that
> were causing actual bugs, but it would be nice to prevent any
> users that are not passing 'unsigned long' arguments.
> 
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.

So you do plan to add some sort of typechecking into IS_ERR_VALUE()?

> Almost all the warnings I got are for files that are better off
> using 'if (err)' or 'if (err < 0)'.
> The only legitimate user I could find that we get a warning for
> is the (32-bit only) freescale fman driver, so I did not remove
> the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
> For 9pfs, I just worked around one user whose calling conventions
> are so obscure that I did not dare change the behavior.
> 
> I was using this definition for testing:
> 
>  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
>unlikely((unsigned long long)(x) >= (unsigned long 
> long)(typeof(x))-MAX_ERRNO))
> 
> which ends up making all 16-bit or wider types work correctly with
> the most plausible interpretation of what IS_ERR_VALUE() was supposed
> to return according to its users, but also causes a compile-time
> warning for any users that do not pass an 'unsigned long' argument.
> 
> I suggested this approach earlier this year, but back then we ended
> up deciding to just fix the users that are obviously broken. After
> the initial warning that caused me to get involved in the discussion
> (fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
> asked me to send the whole thing again.

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Andrew Morton 
On Fri, 27 May 2016 23:23:25 +0200 Arnd Bergmann  wrote:

> Most users of IS_ERR_VALUE() in the kernel are wrong, as they
> pass an 'int' into a function that takes an 'unsigned long'
> argument. This happens to work because the type is sign-extended
> on 64-bit architectures before it gets converted into an
> unsigned type.
> 
> However, anything that passes an 'unsigned short' or 'unsigned int'
> argument into IS_ERR_VALUE() is guaranteed to be broken, as are
> 8-bit integers and types that are wider than 'unsigned long'.
> 
> Andrzej Hajda has already fixed a lot of the worst abusers that
> were causing actual bugs, but it would be nice to prevent any
> users that are not passing 'unsigned long' arguments.
> 
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.

So you do plan to add some sort of typechecking into IS_ERR_VALUE()?

> Almost all the warnings I got are for files that are better off
> using 'if (err)' or 'if (err < 0)'.
> The only legitimate user I could find that we get a warning for
> is the (32-bit only) freescale fman driver, so I did not remove
> the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
> For 9pfs, I just worked around one user whose calling conventions
> are so obscure that I did not dare change the behavior.
> 
> I was using this definition for testing:
> 
>  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
>unlikely((unsigned long long)(x) >= (unsigned long 
> long)(typeof(x))-MAX_ERRNO))
> 
> which ends up making all 16-bit or wider types work correctly with
> the most plausible interpretation of what IS_ERR_VALUE() was supposed
> to return according to its users, but also causes a compile-time
> warning for any users that do not pass an 'unsigned long' argument.
> 
> I suggested this approach earlier this year, but back then we ended
> up deciding to just fix the users that are obviously broken. After
> the initial warning that caused me to get involved in the discussion
> (fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
> asked me to send the whole thing again.

[PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Arnd Bergmann 
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale fman driver, so I did not remove
the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
For 9pfs, I just worked around one user whose calling conventions
are so obscure that I did not dare change the behavior.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
   unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

I suggested this approach earlier this year, but back then we ended
up deciding to just fix the users that are obviously broken. After
the initial warning that caused me to get involved in the discussion
(fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
asked me to send the whole thing again.

Signed-off-by: Arnd Bergmann 
Cc: Andrzej Hajda 
Cc: Andrew Morton 
Link: https://lkml.org/lkml/2016/1/7/363
Link: https://lkml.org/lkml/2016/5/27/486
---
 drivers/acpi/acpi_dbg.c  | 22 +++---
 drivers/ata/sata_highbank.c  |  2 +-
 drivers/clk/tegra/clk-tegra210.c |  2 +-
 drivers/cpufreq/omap-cpufreq.c   |  2 +-
 drivers/crypto/caam/ctrl.c   |  2 +-
 drivers/dma/sun4i-dma.c  | 16 
 drivers/gpio/gpio-xlp.c  |  2 +-
 drivers/gpu/drm/sti/sti_vtg.c|  4 ++--
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  2 +-
 drivers/gpu/host1x/hw/intr_hw.c  |  2 +-
 drivers/iommu/arm-smmu-v3.c  | 18 +-
 drivers/iommu/arm-smmu.c |  8 
 drivers/irqchip/irq-clps711x.c   |  2 +-
 drivers/irqchip/irq-gic.c|  2 +-
 drivers/irqchip/irq-hip04.c  |  2 +-
 drivers/irqchip/spear-shirq.c|  2 +-
 drivers/media/i2c/adp1653.c  | 10 +-
 drivers/media/platform/s5p-tv/mixer_drv.c|  2 +-
 drivers/mfd/twl4030-irq.c|  2 +-
 drivers/mmc/core/mmc.c   |  4 ++--
 drivers/mmc/host/dw_mmc.c|  6 +++---
 drivers/mmc/host/sdhci-esdhc-imx.c   |  2 +-
 drivers/mmc/host/sdhci-of-at91.c |  2 +-
 drivers/mmc/host/sdhci.c |  4 ++--
 drivers/net/ethernet/freescale/fman/fman.c   |  2 +-
 drivers/net/ethernet/freescale/fman/fman_muram.c |  4 ++--
 drivers/net/ethernet/freescale/fman/fman_muram.h |  4 ++--
 drivers/net/wireless/ti/wlcore/spi.c |  4 ++--
 drivers/nvmem/core.c | 22 +++---
 drivers/tty/serial/amba-pl011.c  |  2 +-
 drivers/tty/serial/sprd_serial.c |  2 +-
 drivers/video/fbdev/da8xx-fb.c   |  4 ++--
 fs/afs/write.c   |  4 
 fs/binfmt_flat.c |  6 +++---
 fs/gfs2/dir.c| 15 +--
 kernel/pid.c |  2 +-
 net/9p/client.c  |  4 ++--
 sound/soc/qcom/lpass-platform.c  |  4 ++--
 38 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index 15e4604efba7..1f4128487dd4 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -265,7 +265,7 @@ static int acpi_aml_write_kern(const char *buf, int len)
char *p;
 
ret = acpi_aml_lock_write(crc,

[PATCH] remove lots of IS_ERR_VALUE abuses

2016-05-27 Thread Arnd Bergmann 
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale fman driver, so I did not remove
the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
For 9pfs, I just worked around one user whose calling conventions
are so obscure that I did not dare change the behavior.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
   unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

I suggested this approach earlier this year, but back then we ended
up deciding to just fix the users that are obviously broken. After
the initial warning that caused me to get involved in the discussion
(fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
asked me to send the whole thing again.

Signed-off-by: Arnd Bergmann 
Cc: Andrzej Hajda 
Cc: Andrew Morton 
Link: https://lkml.org/lkml/2016/1/7/363
Link: https://lkml.org/lkml/2016/5/27/486
---
 drivers/acpi/acpi_dbg.c  | 22 +++---
 drivers/ata/sata_highbank.c  |  2 +-
 drivers/clk/tegra/clk-tegra210.c |  2 +-
 drivers/cpufreq/omap-cpufreq.c   |  2 +-
 drivers/crypto/caam/ctrl.c   |  2 +-
 drivers/dma/sun4i-dma.c  | 16 
 drivers/gpio/gpio-xlp.c  |  2 +-
 drivers/gpu/drm/sti/sti_vtg.c|  4 ++--
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  2 +-
 drivers/gpu/host1x/hw/intr_hw.c  |  2 +-
 drivers/iommu/arm-smmu-v3.c  | 18 +-
 drivers/iommu/arm-smmu.c |  8 
 drivers/irqchip/irq-clps711x.c   |  2 +-
 drivers/irqchip/irq-gic.c|  2 +-
 drivers/irqchip/irq-hip04.c  |  2 +-
 drivers/irqchip/spear-shirq.c|  2 +-
 drivers/media/i2c/adp1653.c  | 10 +-
 drivers/media/platform/s5p-tv/mixer_drv.c|  2 +-
 drivers/mfd/twl4030-irq.c|  2 +-
 drivers/mmc/core/mmc.c   |  4 ++--
 drivers/mmc/host/dw_mmc.c|  6 +++---
 drivers/mmc/host/sdhci-esdhc-imx.c   |  2 +-
 drivers/mmc/host/sdhci-of-at91.c |  2 +-
 drivers/mmc/host/sdhci.c |  4 ++--
 drivers/net/ethernet/freescale/fman/fman.c   |  2 +-
 drivers/net/ethernet/freescale/fman/fman_muram.c |  4 ++--
 drivers/net/ethernet/freescale/fman/fman_muram.h |  4 ++--
 drivers/net/wireless/ti/wlcore/spi.c |  4 ++--
 drivers/nvmem/core.c | 22 +++---
 drivers/tty/serial/amba-pl011.c  |  2 +-
 drivers/tty/serial/sprd_serial.c |  2 +-
 drivers/video/fbdev/da8xx-fb.c   |  4 ++--
 fs/afs/write.c   |  4 
 fs/binfmt_flat.c |  6 +++---
 fs/gfs2/dir.c| 15 +--
 kernel/pid.c |  2 +-
 net/9p/client.c  |  4 ++--
 sound/soc/qcom/lpass-platform.c  |  4 ++--
 38 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index 15e4604efba7..1f4128487dd4 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -265,7 +265,7 @@ static int acpi_aml_write_kern(const char *buf, int len)
char *p;
 
ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
-   if (IS_ERR_VALUE(ret))
+   if