Re: [libvirt PATCH v2 08/15] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

On 7/15/20 11:30 AM, Ján Tomko wrote: I disagree with that characterization. The list is created, with 0 elements, when the caller (ebiptablesApplyNewRules()) defines it. Then each time ebtablesGetSubChainInsts() is called, it doesn't create the list anew, it just adds to whatever is already on the existing list - as a matter of fact it is called multiple times and each time it adds more items to the list without re=initializing it. This is very much like what happens with a virBuffer - some function creates a virBuffer by defining it and initializing it to empty, then each time a virBuffer function is called, it adds more text to the buffer. But if there is an error in a virBuffer function, it doesn't clear out the buffer before returning, it just returns an error leaving the buffer in whatever state it was in when the error occurred; it is then up to the caller, who is the owner of the virBuffer, to clear it out. Aside from making the code simpler and cleaner, I think it doesn't make sense for one invocation of the function to clear out anything that was put into the list by *a different* invocation of the function. If you're going to be a purist about it, then a failed ebtablesGetSubChainInsts() should remove from the list *only those items that were added during the current call* and nothing else. But that's just pedantic nitpicking (Hey, *you* started the nitpicking though :-P) (Also, there is only one caller of ebtablesGetSubChainInsts(), and whenever ebtablesGetSubChainInsts() fails, the *very next thing* that caller does is to clear out the entire list. So in fact, "nothing is left allocated in case of failure".) My S-o-b stands. I still think this is the right thing to do.