Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

Andrew Jones
2020-01-24T10:45:50Z

On Fri, Jan 24, 2020 at 10:35:43AM +0100, Andrew Jones wrote:
> > > +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > > +{
> > > + va_list ap;
> > > +
> > > + TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> > > + " num: %u\n",
> > > + num);
>
> Weird line breaking. I see it came from the x86 implementation, but it's
> weird there too... Personally I'd just put it all on one line, because
> my vt100 died two decades ago.
>
> > > +
> > > + va_start(ap, num);
> > > +
> > > + for (i = 0; i < num; i++)
> > > + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
> ^^ should be 'i'

The declaration of 'i' is also missing.

>
> > > + va_arg(ap, uint64_t));
>
> nit: I'd use {} because of the line break. Or just not break the line and
> bust the 80 char "limit" (RIP vt100).
>

Thanks,
drew