Patrick McHardy <kaber@xxxxxxxxx> writes:
> Samir Bellabes wrote:
>> +
>> + msg_head = genlmsg_put(skb_rsp, snet_nl_pid,
>> + atomic_inc_return(&snet_nl_seq),
>> + &snet_genl_family, 0, SNET_C_VERDICT);
>> + if (msg_head == NULL)
>> + goto send_event_failure;
>> +
>> + snet_dbg("verdict_id=0x%x syscall=%s protocol=%u "
>> + "family=%u uid=%u pid=%u\n",
>> + verdict_id, snet_syscall_name(syscall),
>> + protocol, family, current_uid(), current->pid);
>> +
>> + if (verdict_id) {
>> + ret = nla_put_u32(skb_rsp, SNET_A_VERDICT_ID, verdict_id);
>> + if (ret != 0)
>> + goto send_event_failure;
>> + }
>> + ret = nla_put_u8(skb_rsp, SNET_A_SYSCALL, syscall);
>> + if (ret != 0)
>> + goto send_event_failure;
>> + ret = nla_put_u8(skb_rsp, SNET_A_PROTOCOL, protocol);
>> + if (ret != 0)
>> + goto send_event_failure;
>> + ret = nla_put_u8(skb_rsp, SNET_A_FAMILY, family);
>> + if (ret != 0)
>> + goto send_event_failure;
>> + ret = nla_put_u32(skb_rsp, SNET_A_UID, current_uid());
>> + if (ret != 0)
>> + goto send_event_failure;
>> + ret = nla_put_u32(skb_rsp, SNET_A_PID, current->pid);
>> + if (ret != 0)
>> + goto send_event_failure;
>> + ret = nla_put(skb_rsp, SNET_A_DATA_EXT, len, data);
>> + if (ret != 0)
>> + goto send_event_failure;
>
> I guess its a matter of taste, but the NLA_PUT macros already take
> care of exception handling and keep the code smaller.
indeed, I moved the code to use NLA_PUT and the hidden failure label
>> +
>> + ret = genlmsg_end(skb_rsp, msg_head);
>> + if (ret < 0)
>> + goto send_event_failure;
>> +
>> + genlmsg_unicast(&init_net, skb_rsp, snet_nl_pid);
>> + return 0;
>> +
>> +send_event_failure:
>> + kfree_skb(skb_rsp);
>> + return 0;
>
> Shouldn't this error be returned to the caller to avoid waiting
> for the timeout to occur (same question for the return value of
> genlmsg_unicast, which can also fail).
indeed, if error occurred, no need to wait, we can apply the polocy
verdict.
genlmsg_unicast() is taking care of freeing the skb if error occured, so
we can return directly the value of genlmsg_unicast()
thanks Patrick
sam
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
