Re: [PATCH v3] staging: ccree: fixed sparse pointer signedness warnings.

[Gilad Ben-Yossef]

On Wed, Jan 10, 2018 at 3:08 PM, Jeremy Sowden  wrote:
> On 2018-01-10, at 15:03:01 +0200, Gilad Ben-Yossef wrote:
>> On Wed, Jan 10, 2018 at 1:53 PM, Jeremy Sowden wrote:
>> > The driver uses a mixture of signed and unsigned integer variables
>> > for holding arrays lengths and indices, which gives rise to sparse
>> > warnings when the addresses of signed variables are passed to
>> > functions expecting
>>
>> Just curious, I didn't see these warning when running sparse myself.
>> Can you share the command line/flags you've used?
>
> make C=2 CF=-Wsparse-all M=drivers/staging/ccree 2>&1
>

Thanks, I was missing the -Wsparse-all flag.

Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: ccree: fixed sparse pointer signedness warnings.

[Jeremy Sowden]

On 2018-01-10, at 15:03:01 +0200, Gilad Ben-Yossef wrote:
> On Wed, Jan 10, 2018 at 1:53 PM, Jeremy Sowden wrote:
> > The driver uses a mixture of signed and unsigned integer variables
> > for holding arrays lengths and indices, which gives rise to sparse
> > warnings when the addresses of signed variables are passed to
> > functions expecting
>
> Just curious, I didn't see these warning when running sparse myself.
> Can you share the command line/flags you've used?

make C=2 CF=-Wsparse-all M=drivers/staging/ccree 2>&1

Output attached.

> > pointers to unsigned integers.  This patch fixes those warnings by
> > converting those signed variables to unsigned as follows:
> >
> >   * removed some hard-coded array indices.
>
> Removing the hard coded array indices is a separate and different
> issue and so needs to be dealt it in a separate patch. Please split
> your patch into a patchset: one for removing the hard coded values and
> one for the signed/unsigned remedy.

Will do.

J.
[azazel@ulthar:/space/azazel/work/linux/staging] $ git checkout staging-next
Switched to branch 'staging-next'
Your branch is up to date with 'origin/staging-next'.
[azazel@ulthar:/space/azazel/work/linux/staging] $ make C=2 CF=-Wsparse-all 
M=drivers/staging/ccree 2>&1
  CHECK   drivers/staging/ccree/cc_driver.c
  CHECK   drivers/staging/ccree/cc_buffer_mgr.c
drivers/staging/ccree/cc_buffer_mgr.c:1050:46: warning: incorrect type in 
argument 4 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1050:46:expected unsigned int 
[usertype] *lbytes
drivers/staging/ccree/cc_buffer_mgr.c:1050:46:got int *
drivers/staging/ccree/cc_buffer_mgr.c:1083:62: warning: incorrect type in 
argument 7 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1083:62:expected unsigned int 
[usertype] *lbytes
drivers/staging/ccree/cc_buffer_mgr.c:1083:62:got int *
drivers/staging/ccree/cc_buffer_mgr.c:1092:46: warning: incorrect type in 
argument 4 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1092:46:expected unsigned int 
[usertype] *lbytes
drivers/staging/ccree/cc_buffer_mgr.c:1092:46:got int *
drivers/staging/ccree/cc_buffer_mgr.c:1120:49: warning: incorrect type in 
argument 4 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1120:49:expected unsigned int 
[usertype] *src_last_bytes
drivers/staging/ccree/cc_buffer_mgr.c:1120:49:got int *
drivers/staging/ccree/cc_buffer_mgr.c:1121:49: warning: incorrect type in 
argument 5 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1121:49:expected unsigned int 
[usertype] *dst_last_bytes
drivers/staging/ccree/cc_buffer_mgr.c:1121:49:got int *
drivers/staging/ccree/cc_buffer_mgr.c:1124:49: warning: incorrect type in 
argument 2 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1124:49:expected unsigned int 
[usertype] *src_last_bytes
drivers/staging/ccree/cc_buffer_mgr.c:1124:49:got int *
drivers/staging/ccree/cc_buffer_mgr.c:1125:44: warning: incorrect type in 
argument 3 (different signedness)
drivers/staging/ccree/cc_buffer_mgr.c:1125:44:expected unsigned int 
[usertype] *dst_last_bytes
drivers/staging/ccree/cc_buffer_mgr.c:1125:44:got int *
  CC [M]  drivers/staging/ccree/cc_buffer_mgr.o
  CHECK   drivers/staging/ccree/cc_request_mgr.c
  CHECK   drivers/staging/ccree/cc_cipher.c
drivers/staging/ccree/cc_cipher.c:358:17: warning: Variable length array is 
used.
drivers/staging/ccree/cc_cipher.c:700:67: warning: incorrect type in argument 6 
(different signedness)
drivers/staging/ccree/cc_cipher.c:700:67:expected unsigned int *seq_size
drivers/staging/ccree/cc_cipher.c:700:67:got int *
drivers/staging/ccree/cc_cipher.c:703:31: warning: incorrect type in argument 8 
(different signedness)
drivers/staging/ccree/cc_cipher.c:703:31:expected unsigned int *seq_size
drivers/staging/ccree/cc_cipher.c:703:31:got int *
  CC [M]  drivers/staging/ccree/cc_cipher.o
  CHECK   drivers/staging/ccree/cc_hash.c
drivers/staging/ccree/cc_hash.c:480:57: warning: incorrect type in argument 6 
(different signedness)
drivers/staging/ccree/cc_hash.c:480:57:expected unsigned int *seq_size
drivers/staging/ccree/cc_hash.c:480:57:got int *
drivers/staging/ccree/cc_hash.c:530:57: warning: incorrect type in argument 6 
(different signedness)
drivers/staging/ccree/cc_hash.c:530:57:expected unsigned int *seq_size
drivers/staging/ccree/cc_hash.c:530:57:got int *
drivers/staging/ccree/cc_hash.c:1305:43: warning: incorrect type in argument 3 
(different signedness)
drivers/staging/ccree/cc_hash.c:1305:43:expected unsigned int *seq_size
drivers/staging/ccree/cc_hash.c:1305:43:got int *
drivers/staging/ccree/cc_hash.c:1307:43: warning: incorrect type in argument 3 
(different signedness)
drivers/staging/ccree/cc_hash.c:1307:43:expected unsigned int *seq_size
drivers/staging/ccree/cc_hash.c:1307:43:got int *
drivers/staging/ccree/cc_hash.c:1317:69: warning: incorrect type in 

Re: [PATCH v3] staging: ccree: fixed sparse pointer signedness warnings.

[Gilad Ben-Yossef]

Hi Jeremy,

On Wed, Jan 10, 2018 at 1:53 PM, Jeremy Sowden  wrote:
> The driver uses a mixture of signed and unsigned integer variables for
> holding arrays lengths and indices, which gives rise to sparse warnings
> when the addresses of signed variables are passed to functions expecting

Just curious, I didn't see these warning when running sparse myself.
Can you share the command line/flags you've used?

> pointers to unsigned integers.  This patch fixes those warnings by
> converting those signed variables to unsigned as follows:
>
>   * removed some hard-coded array indices.

Removing the hard coded array indices is a separate and different issue
and so needs to be dealt it in a separate patch. Please split your patch
into a patchset: one for removing the hard coded values and one for
the signed/unsigned remedy.

>
>   * changed the types of a number of index and length variables from
> signed to unsigned integer types.
>
>   * changed the return-types of a couple of functions that return length
> values which are assigned to one of these variables from signed to
> unsigned integer types.
>

Other that that it looks good to me.


Thanks!
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: ccree: fixed sparse pointer signedness warnings.

[Jeremy Sowden]

The driver uses a mixture of signed and unsigned integer variables for
holding arrays lengths and indices, which gives rise to sparse warnings
when the addresses of signed variables are passed to functions expecting
pointers to unsigned integers.  This patch fixes those warnings by
converting those signed variables to unsigned as follows:

  * removed some hard-coded array indices.

  * changed the types of a number of index and length variables from
signed to unsigned integer types.

  * changed the return-types of a couple of functions that return length
values which are assigned to one of these variables from signed to
unsigned integer types.

Signed-off-by: Jeremy Sowden 
---
 drivers/staging/ccree/cc_aead.c   | 61 ---
 drivers/staging/ccree/cc_buffer_mgr.c |  2 +-
 drivers/staging/ccree/cc_cipher.c |  3 +-
 drivers/staging/ccree/cc_hash.c   | 17 +-
 4 files changed, 47 insertions(+), 36 deletions(-)

In v3:

 * removed hard-coded index values.
 * more detailed change-log.
 * fixed formatting.

In v2:

 * more detailed change-log.

diff --git a/drivers/staging/ccree/cc_aead.c b/drivers/staging/ccree/cc_aead.c
index 265adffdab41..5a71e014e13c 100644
--- a/drivers/staging/ccree/cc_aead.c
+++ b/drivers/staging/ccree/cc_aead.c
@@ -257,47 +257,55 @@ static void cc_aead_complete(struct device *dev, void 
*cc_req, int err)
aead_request_complete(areq, err);
 }
 
-static int xcbc_setkey(struct cc_hw_desc *desc, struct cc_aead_ctx *ctx)
+static unsigned int xcbc_setkey(struct cc_hw_desc *desc,
+struct cc_aead_ctx *ctx)
 {
+   unsigned int idx = 0;
+
/* Load the AES key */
-   hw_desc_init([0]);
+   hw_desc_init([idx]);
/* We are using for the source/user key the same buffer
 * as for the output keys, * because after this key loading it
 * is not needed anymore
 */
-   set_din_type([0], DMA_DLLI,
+   set_din_type([idx], DMA_DLLI,
 ctx->auth_state.xcbc.xcbc_keys_dma_addr, ctx->auth_keylen,
 NS_BIT);
-   set_cipher_mode([0], DRV_CIPHER_ECB);
-   set_cipher_config0([0], DRV_CRYPTO_DIRECTION_ENCRYPT);
-   set_key_size_aes([0], ctx->auth_keylen);
-   set_flow_mode([0], S_DIN_to_AES);
-   set_setup_mode([0], SETUP_LOAD_KEY0);
-
-   hw_desc_init([1]);
-   set_din_const([1], 0x01010101, CC_AES_128_BIT_KEY_SIZE);
-   set_flow_mode([1], DIN_AES_DOUT);
-   set_dout_dlli([1], ctx->auth_state.xcbc.xcbc_keys_dma_addr,
+   set_cipher_mode([idx], DRV_CIPHER_ECB);
+   set_cipher_config0([idx], DRV_CRYPTO_DIRECTION_ENCRYPT);
+   set_key_size_aes([idx], ctx->auth_keylen);
+   set_flow_mode([idx], S_DIN_to_AES);
+   set_setup_mode([idx], SETUP_LOAD_KEY0);
+   idx++;
+
+   hw_desc_init([idx]);
+   set_din_const([idx], 0x01010101, CC_AES_128_BIT_KEY_SIZE);
+   set_flow_mode([idx], DIN_AES_DOUT);
+   set_dout_dlli([idx], ctx->auth_state.xcbc.xcbc_keys_dma_addr,
  AES_KEYSIZE_128, NS_BIT, 0);
+   idx++;
 
-   hw_desc_init([2]);
-   set_din_const([2], 0x02020202, CC_AES_128_BIT_KEY_SIZE);
-   set_flow_mode([2], DIN_AES_DOUT);
-   set_dout_dlli([2], (ctx->auth_state.xcbc.xcbc_keys_dma_addr
+   hw_desc_init([idx]);
+   set_din_const([idx], 0x02020202, CC_AES_128_BIT_KEY_SIZE);
+   set_flow_mode([idx], DIN_AES_DOUT);
+   set_dout_dlli([idx], (ctx->auth_state.xcbc.xcbc_keys_dma_addr
 + AES_KEYSIZE_128),
  AES_KEYSIZE_128, NS_BIT, 0);
+   idx++;
 
-   hw_desc_init([3]);
-   set_din_const([3], 0x03030303, CC_AES_128_BIT_KEY_SIZE);
-   set_flow_mode([3], DIN_AES_DOUT);
-   set_dout_dlli([3], (ctx->auth_state.xcbc.xcbc_keys_dma_addr
+   hw_desc_init([idx]);
+   set_din_const([idx], 0x03030303, CC_AES_128_BIT_KEY_SIZE);
+   set_flow_mode([idx], DIN_AES_DOUT);
+   set_dout_dlli([idx], (ctx->auth_state.xcbc.xcbc_keys_dma_addr
  + 2 * AES_KEYSIZE_128),
  AES_KEYSIZE_128, NS_BIT, 0);
+   idx++;
 
-   return 4;
+   return idx;
 }
 
-static int hmac_setkey(struct cc_hw_desc *desc, struct cc_aead_ctx *ctx)
+static unsigned int hmac_setkey(struct cc_hw_desc *desc,
+struct cc_aead_ctx *ctx)
 {
unsigned int hmac_pad_const[2] = { HMAC_IPAD_CONST, HMAC_OPAD_CONST };
unsigned int digest_ofs = 0;
@@ -307,7 +315,7 @@ static int hmac_setkey(struct cc_hw_desc *desc, struct 
cc_aead_ctx *ctx)
CC_SHA1_DIGEST_SIZE : CC_SHA256_DIGEST_SIZE;
struct cc_hmac_s *hmac = >auth_state.hmac;
 
-   int idx = 0;
+   unsigned int idx = 0;
int i;
 
/* calc derived HMAC key */
@@ -544,7 +552,8 @@ cc_aead_setkey(struct crypto_aead *tfm, const u8 *key,