On 19.07.23 14:02, Julien Grall wrote:
On 19/07/2023 07:49, Juergen Gross wrote:On 18.07.23 23:35, Julien Grall wrote:Hi Juergen, On 10/07/2023 07:59, Juergen Gross wrote:Today when reading a node from the data base through read_node(), the node data is copied in order to avoid modifying the data base when preparing a node update, as otherwise an error might result in an inconsistent state. There are, however, many cases where such a copy operation isn't needed, as the node isn't modified. Add a "nocopy" flag to read_node() and get_node*() functions for making those cases less memory consuming and more performant.Reducing memory consumption and improving performance is good. However you are now relying on the caller to do the right thing when 'nocopy' is true. I believe this is a disaster waiting to happen.So as it stands, I don't support this approach. The solution I have in mind would require that 'struct node' is const for the 'nocopy' case. I agree this means more work, but that's the price for reduce the the risk of corruption.Fair enough. I'll look into splitting read_node() into a direct variant returning a const pointer and a variant copying the data. Same will be needed for get_node*().Even if this is the "ignore" flag, this is definitely not an ideal situation. And, AFAICT, this is not even document. I don't to be the reader trying to figure out why read_node() and db_fetch() returns a slightly different node content :).Note that there is one modification of the node data left, which is not problematic: domain_adjust_node_perms() might set the "ignore" flag of a permission. This does no harm, as such an update of the permissions doesn't need to be undone in case of a later processing error.So would you be fine with the addition of a comment explaining the situation?I expect that my remark will become moot if we go ahead with splitting read_node().
I have found a sane solution meanwhile. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
