On Mon, Oct 01, 2018 at 02:50 PM +0200, Peter Krempa <pkrempa@xxxxxxxxxx> wrote: > 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@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxx> >> >> >> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx> >> >> >> --- >> >> >> 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. Ahhh okay. Any ideas for the name? Thanks. Side note: What has always surprised me is that the "virDomainDeviceDefPostParse" function is called right out of the context of the function "virDomainDeviceDefParse". Shouldn’t it be called directly _after_ virDomainDeviceDefParse? -- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
