Hi Suraj, > > I don't like this implementation at all. The biggest problem is that you > > are misusing __reassembly(hdev, HCI_ACLDATA_PKT) for getting your SKB. I > > don't wanna intermix this. I am also missing checks for the packet > > length matching or when packets are too big or the header size is not > > matching up. > > > > So in theory both functions do exactly the same. Only minor exception is > > that one knows the packet type up-front, the other has to read it from > > the stream as a 1-byte header. I don't wanna maintain two functions that > > do exactly the same. > > > > Creating an internal helper function that can maintain the current state > > of the reassembly sounds a lot better. Then re-use that function and > > ensure that the reassembly logic is inside the helper. > > I appreciate if you can take a closer look at the patch and compare it > with hci_recv_fragment implementation. > Eventhough it looks similar, there are major differences on the way data > is reassembled. It would not have worked if I had reused the same code > from hci_recv_fragment(). > > Having a common reassembly helper could work. But I am not sure whether > that would be a better solution. I did have a closer look at it already. I clearly see possibilities to combine them into a more generic helper and not to maintain two different functions that do exactly the same. Regards Marcel -- 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
