Skip to contentSkip to conversation

opensm/complib/cl_passivelock.h: some casting issues

PATCH In review Yevgeny Kliteynik 2 participants 3 messages Feb 2, 2010 – Feb 3, 2010 1 message not in the archive InfiniBand

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 opensm/complib/cl_passivelock.h: some casting issues

Sasha,

There are couple of problematic castings in complib/cl_passivelock.h:

(cl_status_t) pthread_rwlock_*()

Turns out that compiler doesn't like casting directly from int to enum, probably because int can be negative:

cl_passivelock.h:199: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:241: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:274: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:306: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status'

Se we need to cast the return value in two stages:
to unsigned, and then to enum.

---

Changes from v2: can't do implicit casting from unsigned
to enum, so specifying it explicitly.

 opensm/include/complib/cl_passivelock.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
opensm/include/complib/cl_passivelock.h +4 -4
index bafd339..aba4e64 100644
@@ -196,7 +196,7 @@ static inline cl_status_t cl_plock_init(IN cl_plock_t * const p_lock)
 	cl_status_t status;
 
 	CL_ASSERT(p_lock);
-	status = (cl_status_t) pthread_rwlock_init(&p_lock->lock, NULL);
+	status = (cl_status_t)(unsigned) pthread_rwlock_init(&p_lock->lock, NULL);
 	if (status)
 		return CL_ERROR;
 	p_lock->state = CL_INITIALIZED;
@@ -238,7 +238,7 @@ static inline void cl_plock_acquire(IN cl_plock_t * const p_lock)
 	CL_ASSERT(p_lock);
 	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_rdlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_rdlock(&p_lock->lock);
 	CL_ASSERT(status == 0);
 }
 
@@ -271,7 +271,7 @@ static inline void cl_plock_excl_acquire(IN cl_plock_t * const p_lock)
 	CL_ASSERT(p_lock);
 	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_wrlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_wrlock(&p_lock->lock);
 	CL_ASSERT(status == 0);
 }
 
@@ -303,7 +303,7 @@ static inline void cl_plock_release(IN cl_plock_t * const p_lock)
 	CL_ASSERT(p_lock);
 	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_unlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_unlock(&p_lock->lock);
 	CL_ASSERT(status == 0);
 }
 
Signature · 5 lines
1.5.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 4B682772.2070304-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

#2 lore raw

Hi Yevgeny,

Quoted 8 lines · Yevgeny Kliteynik

There are couple of problematic castings in complib/cl_passivelock.h:

(cl_status_t) pthread_rwlock_*()

Turns out that compiler doesn't like casting directly from int to enum, probably because int can be negative:

Quoted 7 lines

cl_passivelock.h:199: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:241: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:274: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:306: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status'

Se we need to cast the return value in two stages:
to unsigned, and then to enum.

Instead of bothering with ugly integer casting (and so ugly double/triple/etc casting...) would not it be better to just convert cl_status_t to int?

Something like:

typedef int cl_status_t;

#define CL_SUCCESS 0
#define CL_BLAHBLAH -1
...

Sasha

Quoted 57 lines

---

Changes from v2: can't do implicit casting from unsigned
to enum, so specifying it explicitly.

 opensm/include/complib/cl_passivelock.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
opensm/include/complib/cl_passivelock.h +4 -4
index bafd339..aba4e64 100644
@@ -196,7 +196,7 @@ static inline cl_status_t cl_plock_init(IN cl_plock_t * const p_lock)
 	cl_status_t status;
 
 	CL_ASSERT(p_lock);
-	status = (cl_status_t) pthread_rwlock_init(&p_lock->lock, NULL);
+	status = (cl_status_t)(unsigned) pthread_rwlock_init(&p_lock->lock, NULL);
 	if (status)
 		return CL_ERROR;
 	p_lock->state = CL_INITIALIZED;
@@ -238,7 +238,7 @@ static inline void cl_plock_acquire(IN cl_plock_t * const p_lock)
 	CL_ASSERT(p_lock);
 	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_rdlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_rdlock(&p_lock->lock);
 	CL_ASSERT(status == 0);
 }
 
@@ -271,7 +271,7 @@ static inline void cl_plock_excl_acquire(IN cl_plock_t * const p_lock)
 	CL_ASSERT(p_lock);
 	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_wrlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_wrlock(&p_lock->lock);
 	CL_ASSERT(status == 0);
 }
 
@@ -303,7 +303,7 @@ static inline void cl_plock_release(IN cl_plock_t * const p_lock)
 	CL_ASSERT(p_lock);
 	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_unlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_unlock(&p_lock->lock);
 	CL_ASSERT(status == 0);
 }
 
Signature · 5 lines
1.5.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw

Hi Sasha,

Quoted 16 lines · Sasha Khapyorsky

Hi Yevgeny,

Quoted 13 lines · Yevgeny Kliteynik

There are couple of problematic castings in complib/cl_passivelock.h:

(cl_status_t) pthread_rwlock_*()

Turns out that compiler doesn't like casting directly from int to enum, probably because int can be negative:

cl_passivelock.h:199: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:241: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:274: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status' cl_passivelock.h:306: warning: cast from function call of type 'int' to non-matching type 'enum _cl_status'

Quoted 6 lines
Quoted 2 lines

Se we need to cast the return value in two stages:
to unsigned, and then to enum.

Instead of bothering with ugly integer casting (and so ugly double/triple/etc casting...) would not it be better to just convert cl_status_t to int?

Sure, why not.

-- Yevgeny

Quoted 60 lines

Something like:

typedef int cl_status_t;

#define CL_SUCCESS 0
#define CL_BLAHBLAH -1
...

Sasha

Quoted 50 lines

---

Changes from v2: can't do implicit casting from unsigned
to enum, so specifying it explicitly.

  opensm/include/complib/cl_passivelock.h |    8 ++++----
  1 files changed, 4 insertions(+), 4 deletions(-)
opensm/include/complib/cl_passivelock.h +4 -4
index bafd339..aba4e64 100644
@@ -196,7 +196,7 @@ static inline cl_status_t cl_plock_init(IN cl_plock_t * const p_lock)
  	cl_status_t status;
 
  	CL_ASSERT(p_lock);
-	status = (cl_status_t) pthread_rwlock_init(&p_lock->lock, NULL);
+	status = (cl_status_t)(unsigned) pthread_rwlock_init(&p_lock->lock, NULL);
  	if (status)
  		return CL_ERROR;
  	p_lock->state = CL_INITIALIZED;
@@ -238,7 +238,7 @@ static inline void cl_plock_acquire(IN cl_plock_t * const p_lock)
  	CL_ASSERT(p_lock);
  	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_rdlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_rdlock(&p_lock->lock);
  	CL_ASSERT(status == 0);
  }
 
@@ -271,7 +271,7 @@ static inline void cl_plock_excl_acquire(IN cl_plock_t * const p_lock)
  	CL_ASSERT(p_lock);
  	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_wrlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_wrlock(&p_lock->lock);
  	CL_ASSERT(status == 0);
  }
 
@@ -303,7 +303,7 @@ static inline void cl_plock_release(IN cl_plock_t * const p_lock)
  	CL_ASSERT(p_lock);
  	CL_ASSERT(p_lock->state == CL_INITIALIZED);
 
-	status = (cl_status_t) pthread_rwlock_unlock(&p_lock->lock);
+	status = (cl_status_t)(unsigned) pthread_rwlock_unlock(&p_lock->lock);
  	CL_ASSERT(status == 0);
  }
 
Quoted 8 lines
Quoted 7 lines

--
1.5.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html