diff mbox

[rdma-core] libibumad: clean up htonll/ntohnll handling

Message ID 20170522141722.50502-1-jarod@redhat.com (mailing list archive)
State Accepted
Headers show

Commit Message

Jarod Wilson May 22, 2017, 2:17 p.m. UTC
Only ntohll was being checked to see if it wasn't defined, and was then
redefining htonll as well as ntohll. This was causing some problems for
the compile of the opa-ff package. Simple enough to rearrange this code a
bit such that htonll and ntohll are handled entirely independent of one
another.

Reported-by: Honggang Li <honli@redhat.com>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 libibumad/umad.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Hal Rosenstock May 22, 2017, 3:31 p.m. UTC | #1
On 5/22/2017 10:17 AM, Jarod Wilson wrote:
> Only ntohll was being checked to see if it wasn't defined, and was then
> redefining htonll as well as ntohll. This was causing some problems for
> the compile of the opa-ff package. Simple enough to rearrange this code a
> bit such that htonll and ntohll are handled entirely independent of one
> another.

It used to be the case that they were treated independently.

> Reported-by: Honggang Li <honli@redhat.com>
> Signed-off-by: Jarod Wilson <jarod@redhat.com>

Reviewed-by: Hal Rosenstock <hal@mellanox.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe May 24, 2017, 4:33 p.m. UTC | #2
On Mon, May 22, 2017 at 10:17:22AM -0400, Jarod Wilson wrote:
> Only ntohll was being checked to see if it wasn't defined, and was then
> redefining htonll as well as ntohll. This was causing some problems for
> the compile of the opa-ff package. Simple enough to rearrange this code a
> bit such that htonll and ntohll are handled entirely independent of one
> another.

What problem did this cause?

> +/* Users should use the glibc functions directly, not these wrappers */
>  #ifndef ntohll
> -#undef htonll
>  #undef ntohll
> -/* Users should use the glibc functions directly, not these wrappers */
> -static inline __attribute__((deprecated)) uint64_t htonll(uint64_t x) { return htobe64(x); }
>  static inline __attribute__((deprecated)) uint64_t ntohll(uint64_t x) { return be64toh(x); }
> -#define htonll htonll
>  #define ntohll ntohll
>  #endif
> +#ifndef htonll
> +#undef htonll

Both these undefs are not needed anymore.

Seems fine to me, if not weird that something would have a problem
here.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarod Wilson May 27, 2017, 10:01 p.m. UTC | #3
On 2017-05-24 12:33 PM, Jason Gunthorpe wrote:
> On Mon, May 22, 2017 at 10:17:22AM -0400, Jarod Wilson wrote:
>> Only ntohll was being checked to see if it wasn't defined, and was then
>> redefining htonll as well as ntohll. This was causing some problems for
>> the compile of the opa-ff package. Simple enough to rearrange this code a
>> bit such that htonll and ntohll are handled entirely independent of one
>> another.
> 
> What problem did this cause?
> 
>> +/* Users should use the glibc functions directly, not these wrappers */
>>   #ifndef ntohll
>> -#undef htonll
>>   #undef ntohll
>> -/* Users should use the glibc functions directly, not these wrappers */
>> -static inline __attribute__((deprecated)) uint64_t htonll(uint64_t x) { return htobe64(x); }
>>   static inline __attribute__((deprecated)) uint64_t ntohll(uint64_t x) { return be64toh(x); }
>> -#define htonll htonll
>>   #define ntohll ntohll
>>   #endif
>> +#ifndef htonll
>> +#undef htonll
> 
> Both these undefs are not needed anymore.
> 
> Seems fine to me, if not weird that something would have a problem
> here.

My understanding from Honggang is that this was necessary for him to get 
opa-ff and infiniband-diags to compile (along with an update to our 
libibmad package). He should be able to fill in the blanks with further 
details.
Leon Romanovsky May 30, 2017, 3:52 a.m. UTC | #4
On Mon, May 22, 2017 at 10:17:22AM -0400, Jarod Wilson wrote:
> Only ntohll was being checked to see if it wasn't defined, and was then
> redefining htonll as well as ntohll. This was causing some problems for
> the compile of the opa-ff package. Simple enough to rearrange this code a
> bit such that htonll and ntohll are handled entirely independent of one
> another.
>
> Reported-by: Honggang Li <honli@redhat.com>
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
>  libibumad/umad.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>

Thanks, applied.
diff mbox

Patch

diff --git a/libibumad/umad.h b/libibumad/umad.h
index 81811380..479165a8 100644
--- a/libibumad/umad.h
+++ b/libibumad/umad.h
@@ -247,15 +247,17 @@  static inline void umad_free(void *umad)
 	free(umad);
 }
 
+/* Users should use the glibc functions directly, not these wrappers */
 #ifndef ntohll
-#undef htonll
 #undef ntohll
-/* Users should use the glibc functions directly, not these wrappers */
-static inline __attribute__((deprecated)) uint64_t htonll(uint64_t x) { return htobe64(x); }
 static inline __attribute__((deprecated)) uint64_t ntohll(uint64_t x) { return be64toh(x); }
-#define htonll htonll
 #define ntohll ntohll
 #endif
+#ifndef htonll
+#undef htonll
+static inline __attribute__((deprecated)) uint64_t htonll(uint64_t x) { return htobe64(x); }
+#define htonll htonll
+#endif
 
 END_C_DECLS
 #endif				/* _UMAD_H */