Hi Dan,
>I hate these patches. I have told Markus to stop sending them but he
>has issues so now I only complain when they introduce a bug. There was
>one bug I have missed because it was a benchmark regression and I knew
>it was theoretically possible but I didn't know the code well enough to
>say which were fast paths...
>My main objection is that relying on the sanity check inside the
>function call makes the code more subtle to understand. We know we need
>a NULL check but it is hidden away in another file. The motivation for
>this patch you are sending is "There is a sanity check in dev_kfree_skb()
>so let's do an insane thing and save some lines of code."
>For this particular patch we assume throughout the whole driver that
>"pTDInfo->skb" can be NULL so making it inconsistent in this one place
>is wrong
Agreed,
But these changes are suggested because:-
where we are checking for (pTDInfo->skb), we are using it in above line.
and it does not look good, thats why we should remove thse checks and i have suggested
changes.
code snippet:-
-----------------------
if (pTDInfo->skb_dma && (pTDInfo->skb_dma != pTDInfo->buf_dma))
dma_unmap_single(&pDevice->pcid->dev, pTDInfo->skb_dma,
pTDInfo->skb->len, DMA_TO_DEVICE);
----> In this we did not check for pTDInfo->skb
if (pTDInfo->skb)
dev_kfree_skb(pTDInfo->skb);
But if am wrong, sorry for the patch.
Thanks,
Maninder
............
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
