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.)