On 2018-08-20 17:13, Vladimir Sementsov-Ogievskiy wrote: > 20.08.2018 16:44, Max Reitz wrote: >> On 2018-08-20 12:20, Vladimir Sementsov-Ogievskiy wrote:
[...] >>> My goal is to get graphviz representation of block graph with all the >>> parents for debugging. And I'm absolutely ok to do it with x-debug-. >>> Then we shouldn't care about enum for role type now. So, it the patch ok >>> for you with x-debug- prefix? >> Actually, no, because I'm not sure whether using points for the IDs is a >> good idea. That may defeat ASLR, and that would be a problem even with >> x-debug-. > > Good point, agree. > >> >> So I'd prefer using e.g. a hash map to map pointers to freshly generated >> IDs (just incrementing integers). (By the way, that would also improve the speed of checking whether a certain node needs to be added to the @nodes list still.) >> In any case, I'll take a further look at the patch later, but another >> thing that I've just seen is that using the opaque pointers to identify >> a parent is something that doesn't look like it's guaranteed to work. > > Hm, isn't it a bug? Can you point to an example? Ah, no, it's OK. Well, kind of. It's OK in the sense that it is unique when set. I didn't notice that you only use it for the non-node parents, sorry. Still, it probably would be better to just use the BdrvChild object, as that should be unique as well, and it is obviously non-NULL. (BdrvChild.opaque may be NULL, even though it isn't in practice.) Max
signature.asc
Description: OpenPGP digital signature
