[dpdk-dev,v3,11/24] rte_mbuf.h: avoid warnings from inadvertant promotion

Message ID 152609037755.121661.8441874507300238825.stgit@localhost.localdomain (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Andy Green May 12, 2018, 1:59 a.m. UTC
  Signed-off-by: Andy Green <andy@warmcat.com>
---
 lib/librte_mbuf/rte_mbuf.h |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
  

Comments

Thomas Monjalon May 13, 2018, 4:54 p.m. UTC | #1
12/05/2018 03:59, Andy Green:
> @@ -836,8 +836,9 @@ rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
>  	 * reference counter can occur.
>  	 */
>  	if (likely(rte_mbuf_refcnt_read(m) == 1)) {
> -		rte_mbuf_refcnt_set(m, 1 + value);
> -		return 1 + value;
> +		++value;
> +		rte_mbuf_refcnt_set(m, value);
> +		return value;
>  	}

I don't understand what it is fixing.
Please could you explain or show the GCC warning?
  
Andy Green May 13, 2018, 11:27 p.m. UTC | #2
On 05/14/2018 12:54 AM, Thomas Monjalon wrote:
> 12/05/2018 03:59, Andy Green:
>> @@ -836,8 +836,9 @@ rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
>>   	 * reference counter can occur.
>>   	 */
>>   	if (likely(rte_mbuf_refcnt_read(m) == 1)) {
>> -		rte_mbuf_refcnt_set(m, 1 + value);
>> -		return 1 + value;
>> +		++value;
>> +		rte_mbuf_refcnt_set(m, value);
>> +		return value;
>>   	}
> 
> I don't understand what it is fixing.
> Please could you explain or show the GCC warning?

"1 + value", where value is an uint16_t causes promotion to a signed 
int.  The compiler complained that we are shoving an int into a uint16_t 
return type with different size and sign.

Bumping and returning value directly instead removes the promotion and 
the problem.

I added this explanation to the patch (although the title gave a big 
clue already).

-Andy

>
  

Patch

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 4fd9a0d9e..a2a37a311 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -836,8 +836,9 @@  rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
 	 * reference counter can occur.
 	 */
 	if (likely(rte_mbuf_refcnt_read(m) == 1)) {
-		rte_mbuf_refcnt_set(m, 1 + value);
-		return 1 + value;
+		++value;
+		rte_mbuf_refcnt_set(m, value);
+		return value;
 	}
 
 	return __rte_mbuf_refcnt_update(m, value);
@@ -927,8 +928,9 @@  rte_mbuf_ext_refcnt_update(struct rte_mbuf_ext_shared_info *shinfo,
 	int16_t value)
 {
 	if (likely(rte_mbuf_ext_refcnt_read(shinfo) == 1)) {
-		rte_mbuf_ext_refcnt_set(shinfo, 1 + value);
-		return 1 + value;
+		++value;
+		rte_mbuf_ext_refcnt_set(shinfo, value);
+		return value;
 	}
 
 	return (uint16_t)rte_atomic16_add_return(&shinfo->refcnt_atomic, value);