re: #25494 (review)
In commit “interfaces, refactor: Add more block information to block connected notifications” (0a93a16614659a7c12e7e95478a9bb564c1a13cb)
It seems that this will only copy/alias the internal memory to a different type. I wonder if this is actually useful, because it disables lock annotations for the memory where they may actually still be needed?
I don’t think there’s actually a concern here, and this is what current code already does. cs_main is locked while reading nFile and nDataPos values, but not when ReadBlockFromDisk is called. Values are copied first and then used.
Also, I wonder how useful an interface is that returns a data structure that points into memory that is owned by a different process (I am assuming each blockindex runs in a different multiprocess)
The change in this commit is to replace individual block & height parameters with a BlockInfo struct parameter that can hold more information about the block:
0- virtual void blockConnected(const CBlock& block, int height) {}
1- virtual void blockDisconnected(const CBlock& block, int height) {}
2+ virtual void blockConnected(const BlockInfo& block) {}
3+ virtual void blockDisconnected(const BlockInfo& block) {}
The point of using the struct is to make code more readable and avoid the need to update blockConnected/blockDisconnected declarations when more block information is needed.
Semantically there’s no difference between passing a const uint256& hash parameter vs adding a const uint256& hash; struct member, or passing an int file_number parameter vs. adding an int file_number; struct member. All the copy and reference and lifetime concerns are the same.
It is true that because referencing memory in other processes is not possible, multiprocess code will create local copies of values when references are passed, and the local copies may have shorter lifetimes than original references. But this is true wherever references and pointers are used in src/interface/ classes, so not a new thing.
