Re: [PATCH] Add const qualifier in rdma_reg*, rdma_post_send*|write and ibv_reg_mr (was: Re: non-const pointer void * addr in rdma_reg_* and rdma_post_[send|write])
Hi, Le jeudi 28 novembre 2013 à 20:47 +0100, Hannes Weisbach a écrit : > > But adding "const" should not break compilation of existing userspace > > program. > That was my thought too. However, because those functions are defined > in as inline in a header, they actually do break user code. For > example, rdma_reg_msgs passes a const void * to ibv_reg_mr, which > takes a void *, thus resulting in a warning. If you (like me) compile > with the -Werror flag, this results in broken code. The only > offending libibverbs function I found so far is ibv_reg_mr(). > > > > PS: this issue could also be raised against libibverbs, so once you've > > fixed librdmacm, why not fix libibverbs after ? > This was my plan. But first I wanted to see if there even is interest > in fixing this. As a user I deal with librdmacm directly, so I > started to fix it first. > > Anyway, since patching librdmacm alone would break user code, I also > included a patch for libibverbs, which adds the const qualifier to > 'addr' parameter ib ibv_reg_mr(). > To avoid adding cast to non-const in librdmacm that no one will remove once added, you're correct in patching libibverbs first. > lists.openfabrics.org says, this is also the mailing list for > libibverbs (README is outdated ;)), I hope this is ok. > It's OK. Roland Dreier, libibverbs maintainer, is here (but sadly not as often as we would like). Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Add const qualifier in rdma_reg*, rdma_post_send*|write and ibv_reg_mr (was: Re: non-const pointer void * addr in rdma_reg_* and rdma_post_[send|write])
Hi. > Two days ago I started to work on a kernel patchset to address similar > concerns on the verbs/RDMA APIs Nice. > > BTW, it's easier to change the kernel "internal" API than the public > userspace API of the library. But adding "const" should not break > compilation of existing userspace program. That was my thought too. However, because those functions are defined in as inline in a header, they actually do break user code. For example, rdma_reg_msgs passes a const void * to ibv_reg_mr, which takes a void *, thus resulting in a warning. If you (like me) compile with the -Werror flag, this results in broken code. The only offending libibverbs function I found so far is ibv_reg_mr(). > > PS: this issue could also be raised against libibverbs, so once you've > fixed librdmacm, why not fix libibverbs after ? This was my plan. But first I wanted to see if there even is interest in fixing this. As a user I deal with librdmacm directly, so I started to fix it first. Anyway, since patching librdmacm alone would break user code, I also included a patch for libibverbs, which adds the const qualifier to 'addr' parameter ib ibv_reg_mr(). lists.openfabrics.org says, this is also the mailing list for libibverbs (README is outdated ;)), I hope this is ok. Best regards, Hannes 0001-Make-addr-parameter-of-rdma_reg_-and-rdma_post_send-.patch Description: Binary data 0001-Make-ibv_mr_reg-parameter-void-addr-const.patch Description: Binary data
Re: non-const pointer void * addr in rdma_reg_* and rdma_post_[send|write]
Hi, Le mercredi 27 novembre 2013 à 19:00 +0100, Hannes Weisbach a écrit : > I just started working with librdmacm and I was wondering if there is a > specific reason why rdma_reg_* functions and rdma_post_send/write functions > take the local memory address as non-const pointer "void * addr". These > functions shouldn't and don't change the memory pointed to by addr. I think > this should be made explicit by using the type const void * for addr. > In case you agree, I would volunteer to make the necessary changes. > :) Two days ago I started to work on a kernel patchset to address similar concerns on the verbs/RDMA APIs BTW, it's easier to change the kernel "internal" API than the public userspace API of the library. But adding "const" should not break compilation of existing userspace program. PS: this issue could also be raised against libibverbs, so once you've fixed librdmacm, why not fix libibverbs after ? Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: non-const pointer void * addr in rdma_reg_* and rdma_post_[send|write]
> I just started working with librdmacm and I was wondering if there is a > specific reason why rdma_reg_* functions and rdma_post_send/write functions > take the local memory address as non-const pointer "void * addr". These > functions shouldn't and don't change the memory pointed to by addr. I think > this should be made explicit by using the type const void * for addr. > In case you agree, I would volunteer to make the necessary changes. The librdmacm calls simply abstract the libibverbs ibv_post_send() call. I agree that making them const makes sense, but the inline code would simply cast away the const anyway. Either way seems fine with me. If you submitted a patch, I would accept it. - Sean -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
non-const pointer void * addr in rdma_reg_* and rdma_post_[send|write]
Hi, I just started working with librdmacm and I was wondering if there is a specific reason why rdma_reg_* functions and rdma_post_send/write functions take the local memory address as non-const pointer "void * addr". These functions shouldn't and don't change the memory pointed to by addr. I think this should be made explicit by using the type const void * for addr. In case you agree, I would volunteer to make the necessary changes. Best regards, Hannes Weisbach-- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
