Re: [libvirt] [PATCH 06/11] conf: Get rid of virDomainDeviceDefPostParseOne

On Mon, Oct 01, 2018 at 14:38:40 +0200, Marc Hartmayer wrote: > On Mon, Oct 01, 2018 at 12:41 PM +0200, Peter Krempa <pkrempa(a)redhat.com&gt; wrote: > > On Mon, Oct 01, 2018 at 12:27:41 +0200, Marc Hartmayer wrote: > >> On Sat, Sep 29, 2018 at 04:09 AM +0200, John Ferlan <jferlan(a)redhat.com&gt; wrote: > >> > On 9/20/18 1:44 PM, Marc Hartmayer wrote: > >> >> Move the domainPostParseDataAlloc/Free calls to > >> >> virDomainDeviceDefParse. As an consequence > >> >> virDomainDeviceDefPostParseOne is superfluous and can therefore be > >> >> removed. > >> >> > >> >> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com&gt; > >> >> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > >> >> --- > >> >> src/conf/domain_conf.c | 37 +++++++++++-------------------------- > >> >> 1 file changed, 11 insertions(+), 26 deletions(-) > >> >> > >> > > >> > I'm not against this per se; however, I should not that the code was > >> > specifically extracted in commit e168bc8a. > >> > >> There are the following three functions: > >> > >> virDomainDeviceDefParse > >> virDomainDeviceDefPostParse > >> virDomainDeviceDefPostParseOne > >> > >> Peter introduced the function “virDomainDeviceDefPostParseOne” to avoid > >> the allocation of private data across the callbacks. This is absolutely > >> fine. > >> > >> What I’ve done is, I moved the domainPostParseDataAlloc/Free calls to > >> virDomainDeviceDefParse instead of having an extra wrapper function > >> (virDomainDeviceDefPostParse/One) for that. With this change I can reuse > >> the QEMU caps for the virDomainDeviceDefValidate call in > >> virDomainDeviceDefParse as well. > > > > For the above it's not necessary to open-code what virDomainDeviceDefPostParseOne > > in a rather massive function. You can pass the opaque in if you want. > > I don’t get it. Where can I pass the opaque? virDomainDeviceDefParse is a big spaghettified function and adding other code to it is unwanted. If you need to unify the allocation of private data, move the validation function call to the helper (with appropriate rename) rather than blatantly pasting the code back.