On Fri, 29 Nov 2024 at 15:29, Cyril Bur <cyrilbur@xxxxxxxxxxxxxxx> wrote:
> --- a/riscv/kvm-cpu.c
> +++ b/riscv/kvm-cpu.c
> @@ -178,15 +178,16 @@ static bool kvm_cpu_riscv_sbi(struct kvm_cpu *vcpu)
> break;
> }
> vcpu->kvm_run->riscv_sbi.ret[1] = 0;
> - while (str_start <= str_end) {
> - if (vcpu->kvm_run->riscv_sbi.function_id ==
> - SBI_EXT_DBCN_CONSOLE_WRITE) {
> - term_putc(str_start, 1, 0);
> - } else {
> - if (!term_readable(0))
> - break;
> - *str_start = term_getc(vcpu->kvm, 0);
> - }
> + if (vcpu->kvm_run->riscv_sbi.function_id ==
> + SBI_EXT_DBCN_CONSOLE_WRITE) {
> + int length = (str_end - str_start) + 1;
Much more readable than v1!
You could add a check that str_end > str_start where the if
(!str_start || !str_end) test is to avoid shenanigans.
> +
> + term_putc(str_start, length, 0);
> + vcpu->kvm_run->riscv_sbi.ret[1] += length;
term_putc returns the actual length written, should you be putting
that in ret[1] instead?
> + break;
> + }
> + while (str_start <= str_end && term_readable(0)) {
Previously this would have only happened for function_id ==
SBI_EXT_DBCN_CONSOLE_READ. Should you add an else?
> + *str_start = term_getc(vcpu->kvm, 0);
> vcpu->kvm_run->riscv_sbi.ret[1]++;
> str_start++;
> }
> --
> 2.34.1
>
