Hi Arman, >>> + assert(notify_data->chrc->ccc_handle); >> >> You are going heavy on the asserts. I am not convinced that is always a good idea. If remote devices can exploit such an assert, we have a problem. Gracefully disconnect might be better in many cases. >> > > There are certain invariants that go with how the reference count is > managed and I chose to add asserts in certain places to help catch > potential bugs. I used them only for things that should never ever > fail: i.e. if any of these assertions fails then there is a bug in the > code and we should fix them. I don't mind removing them, but all the > asserts that I added here (and in the previous patch that you > responded to) try to capture the logical state and assumptions that > are made in the code from which there isn't really a good way to > recover if these assumptions do not hold. > > In short, these assertions should always succeed in non-buggy code. > Does BlueZ have debug-mode-only assert macros? Maybe such a thing > might be useful to make the code self document these assumptions. > Otherwise, I can remove them and replace them with comments instead? actually that is a good idea. We might should look into introducing some nice and handy asserts helpers for exactly the case when you want it on during development and testing. For now, just keep the asserts in it. I just wanted to point it out since I have seen people going crazy with asserts and then they can be remotely exploited. And that is something I want to avoid at all costs. A remote side should never ever be able to trigger an assert. 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
