----- On Feb 2, 2017, at 3:19 PM, pablo pa...@netfilter.org wrote:
> On Mon, Jan 30, 2017 at 05:37:10PM +0100, Andreas Schultz wrote: >> This unifies duplicate code into a helper. It also prepares the >> groundwork to add a lookup version that uses the socket to find >> attache pdp contexts. >> >> Signed-off-by: Andreas Schultz <aschu...@tpip.net> >> --- >> drivers/net/gtp.c | 120 >> +++++++++++++++++++++++------------------------------- >> 1 file changed, 51 insertions(+), 69 deletions(-) >> >> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c >> index c96c71f..6b7a3c2 100644 >> --- a/drivers/net/gtp.c >> +++ b/drivers/net/gtp.c > [...] >> +static struct pdp_ctx *gtp_genl_find_pdp(struct sk_buff *skb, >> + struct genl_info *info) >> +{ >> + struct pdp_ctx *pctx; >> + >> + if (info->attrs[GTPA_LINK]) >> + pctx = gtp_genl_find_pdp_by_link(skb, info); >> + else >> + pctx = ERR_PTR(-EINVAL); >> + if (!pctx) >> + pctx = ERR_PTR(-ENOENT); >> + >> + return pctx; >> +} > > For gtp_genl_find_pdp(), I think this is easier to read: > > if (!info->attrs[GTPA_LINK]) > return ERR_PTR(-EINVAL); > > pctx = gtp_genl_find_pdp_by_link(skb, info); > if (!pctx) > return ERR_PTR(-ENOENT); > > return pctx; Yes, but a later patch (will be submitted after this series is accepted) will change that to: if (info->attrs[GTPA_LINK]) pctx = gtp_genl_find_pdp_by_link(skb, info); else if (info->attrs[GTPA_FD]) pctx = gtp_genl_find_pdp_by_socket(skb, info); else pctx = ERR_PTR(-EINVAL); if (!pctx) pctx = ERR_PTR(-ENOENT); return pctx; I can use your form for this change, but have a larger change later. Which way do you prefer it? Andreas