Hi Martin,
On Tue, Sep 16, 2014 at 03:44:43PM +0100, Martin Townsend wrote:
> I would like to keep freeing skb's out of process_data as process_data will become something like iphc_decompress_hdr and it would be good if that's all it did. Otherwise I feel we are going to put a constraint on all future header decompression routines in that they must free the skb on error. I think it would be better to defer this so on error you might want to try something else with the skb, maybe not but at least the option is there.
> So how about
>
> struct sk_buff * ret_skb;
> switch (skb->data[0] & 0xe0) {
> case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */
> ret_skb = process_data(skb, &hdr);
> if (IS_ERR(ret_skb))
> goto drop_skb;
> else
> skb = ret_skb;
> break;
>
> I know we currently have 3 calls to process_data so it will look fairly ugly in this patch but in my next patch to fix lowpan_rcv to handle uncompressed IPv6 packets that are fragmented there will only be one call to process_data so it won't look so bad. You could even wrap it in a macro but I'm not a fan of this as they can obfuscate the code a bit.
>
> Thoughts?
>
sorry, I can't follow how this solve the issue if the "parameter skb" is
already consumed or not. If process_data returns a error before
parameter consume, then we should run kfree_skb(parameter_skb), if it's
afterwards we should do nothing. Point is we don't know that there. I
suppose if we do consume_skb and refcount reach 0 the parameter_skb
becomes a dangling pointer.
- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
