Re: [PULL 03/11] tools: build qemu-vmsr-helper

[Paolo Bonzini]

On Thu, Jul 25, 2024 at 12:28 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > +    if (r < 0) {
> > +        error_report_err(local_err);
> > +        goto out;
>
> Here we have a check for r < 0 that forces an early exit...
>
> > +    }
> > +
> > +    while (r < 0) {
>
> ...but then immediately we do a while (r < 0). r cannot be < 0
> here because we just checked that, so this while loop will
> never execute and the whole loop body is dead code.
>
> What was the intention here ?

The intention was to have "while (r >= 0)" which is effectively an
infinite loop.

Paolo

>
>
> > +        /*
> > +         * Read the requested MSR
> > +         * Only RAPL MSR in rapl-msr-index.h is allowed
> > +         */
> > +        r = qio_channel_read_all(QIO_CHANNEL(client->ioc),
> > +                                (char *) &request, sizeof(request), 
> > &local_err);
> > +        if (r < 0) {
> > +            error_report_err(local_err);
> > +            break;
> > +        }
> > +
> > +        if (!is_msr_allowed(request[0])) {
> > +            error_report("Requested unallowed msr: %d", request[0]);
> > +            break;
> > +        }
> > +
> > +        vmsr = vmsr_read_msr(request[0], request[1]);
> > +
> > +        if (!is_tid_present(peer_pid, request[2])) {
> > +            error_report("Requested TID not in peer PID: %d %d",
> > +                peer_pid, request[2]);
> > +            vmsr = 0;
> > +        }
> > +
> > +        r = qio_channel_write_all(QIO_CHANNEL(client->ioc),
> > +                                  (char *) &vmsr,
> > +                                  sizeof(vmsr),
> > +                                  &local_err);
> > +        if (r < 0) {
> > +            error_report_err(local_err);
> > +            break;
> > +        }
> > +    }
> > +out:
> > +    object_unref(OBJECT(client->ioc));
> > +    g_free(client);
> > +}
>
> thanks
> -- PMM
>