On Wed, Mar 13, 2019 at 10:18:52 +0100, Andrea Bolognani wrote: > On Wed, 2019-03-13 at 09:41 +0100, Peter Krempa wrote: > > On Thu, Mar 07, 2019 at 16:44:32 +0100, Andrea Bolognani wrote: > > > This removes a little duplication right away, and more > > > importantly will allow us to perform some interesting > > > refactoring later on. > [...] > > > @@ -47,6 +48,8 @@ testQemuDataInit(testQemuDataPtr data) > > > if (qemuTestDriverInit(&data->driver) < 0) > > > return -1; > > > > > > + data->dataDir = abs_srcdir "/qemucapabilitiesdata"; > > > + > > > > I'm not entirely persuaded that you need to pass this constant in via > > the data structure. Even after the last patch the value is never > > modified. Could you please elaborate on the refactorings that this will > > allow? > > I'll admit I kinda copy-pasted commit messages around, so that's > probably why the one for this patch sounds a bit hyperbolic :) > > Anyway, by the end of the series we'll have to pass dataDir to > testQemuCapsIterate() in addition to using it to build the path for > both the input and output file, so it makes sense to store it into > a variable instead of building it from abs_srcdir plus the directory > name two or more times. > > As for why it's stored in the structure, I could easily have used a > global variable instead, but this approach seemed cleaner, especially > considering that we're already using the structure to store data that > is similar in scope (the driver). Well, I agree that we should pre-construct the string rather than constructing it multiple times, but since every use of that string is inside the function which does the testing there's no point passing it in via the structure.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
