| Message ID | 1522469906-4745-1-git-send-email-yanjun.zhu@oracle.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Sat, 2018-03-31 at 00:18 -0400, Zhu Yanjun wrote: > -#if IS_ENABLED(CONFIG_IPV6) > static struct dst_entry *rxe_find_route6(struct net_device *ndev, > struct in6_addr *saddr, > struct in6_addr *daddr) > { > +#if IS_ENABLED(CONFIG_IPV6) > struct dst_entry *ndst; > struct flowi6 fl6 = { { 0 } }; > > @@ -168,20 +168,10 @@ static struct dst_entry *rxe_find_route6(struct net_device *ndev, > return ndst; > put: > dst_release(ndst); > +#endif > return NULL; > } > > -#else > - > -static struct dst_entry *rxe_find_route6(struct net_device *ndev, > - struct in6_addr *saddr, > - struct in6_addr *daddr) > -{ > - return NULL; > -} > - > -#endif Is this patch more than code churn? This patch changes code that conforms to the kernel coding standard into code that no longer conforms to the kernel coding standard. A quote from Documentation/process/coding-style.rst: "Prefer to compile out entire functions, rather than portions of functions or portions of expressions. Rather than putting an ifdef in an expression, factor out part or all of the expression into a separate helper function and apply the conditional to that function." Hence please drop this patch. Thanks, Bart.
On Sat, Mar 31, 2018 at 04:40:05AM +0000, Bart Van Assche wrote: > On Sat, 2018-03-31 at 00:18 -0400, Zhu Yanjun wrote: > > -#if IS_ENABLED(CONFIG_IPV6) > > static struct dst_entry *rxe_find_route6(struct net_device *ndev, > > struct in6_addr *saddr, > > struct in6_addr *daddr) > > { > > +#if IS_ENABLED(CONFIG_IPV6) > > struct dst_entry *ndst; > > struct flowi6 fl6 = { { 0 } }; > > > > @@ -168,20 +168,10 @@ static struct dst_entry *rxe_find_route6(struct net_device *ndev, > > return ndst; > > put: > > dst_release(ndst); > > +#endif > > return NULL; > > } > > > > -#else > > - > > -static struct dst_entry *rxe_find_route6(struct net_device *ndev, > > - struct in6_addr *saddr, > > - struct in6_addr *daddr) > > -{ > > - return NULL; > > -} > > - > > -#endif > > Is this patch more than code churn? > > This patch changes code that conforms to the kernel coding standard into > code that no longer conforms to the kernel coding standard. A quote from > Documentation/process/coding-style.rst: "Prefer to compile out entire > functions, rather than portions of functions or portions of > expressions. Rather than putting an ifdef in an expression, factor out part > or all of the expression into a separate helper function and apply the > conditional to that function." Yes, if the headers don't support using the 'if (IS_ENABLED(..))' then as-is is best. 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
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c index 159246b..4fc3941 100644 --- a/drivers/infiniband/sw/rxe/rxe_net.c +++ b/drivers/infiniband/sw/rxe/rxe_net.c @@ -140,11 +140,11 @@ static struct dst_entry *rxe_find_route4(struct net_device *ndev, return &rt->dst; } -#if IS_ENABLED(CONFIG_IPV6) static struct dst_entry *rxe_find_route6(struct net_device *ndev, struct in6_addr *saddr, struct in6_addr *daddr) { +#if IS_ENABLED(CONFIG_IPV6) struct dst_entry *ndst; struct flowi6 fl6 = { { 0 } }; @@ -168,20 +168,10 @@ static struct dst_entry *rxe_find_route6(struct net_device *ndev, return ndst; put: dst_release(ndst); +#endif return NULL; } -#else - -static struct dst_entry *rxe_find_route6(struct net_device *ndev, - struct in6_addr *saddr, - struct in6_addr *daddr) -{ - return NULL; -} - -#endif - static struct dst_entry *rxe_find_route(struct rxe_dev *rxe, struct rxe_qp *qp, struct rxe_av *av)
In the file rxe_net.c, to make rxe_find_route6 compact, IPV6_CONFIG is moved into the function rxe_find_route6. CC: Srinivas Eeda <srinivas.eeda@oracle.com> CC: Junxiao Bi <junxiao.bi@oracle.com> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com> --- V2->V3: Fix compile error. V1->V2: Follow Jason's advice, remove #if. --- drivers/infiniband/sw/rxe/rxe_net.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)