qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-4.1 1/4] hw/isa/superio: Rename a variable


From: Thomas Huth
Subject: Re: [Qemu-devel] [PATCH for-4.1 1/4] hw/isa/superio: Rename a variable
Date: Fri, 5 Apr 2019 06:14:26 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

On 05/04/2019 00.12, Philippe Mathieu-Daudé wrote:
> This patch is purely cosmetic. No functional change.
> This will ease the next patch where we re-indent an if() statement.
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/isa/isa-superio.c | 69 ++++++++++++++++++++++----------------------
>  1 file changed, 34 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
> index d54463bf035..c6845eaf578 100644
> --- a/hw/isa/isa-superio.c
> +++ b/hw/isa/isa-superio.c
> @@ -22,8 +22,8 @@
>  
>  static void isa_superio_realize(DeviceState *dev, Error **errp)
>  {
> -    ISASuperIODevice *sio = ISA_SUPERIO(dev);
> -    ISASuperIOClass *k = ISA_SUPERIO_GET_CLASS(sio);
> +    ISASuperIODevice *s = ISA_SUPERIO(dev);
> +    ISASuperIOClass *k = ISA_SUPERIO_GET_CLASS(s);

Sorry, but I have to say that I normally really don't like single-letter
variable names. They are much harder to search for, and way less
self-describing.

If you get problems with the line length in a later patch, what about
refactoring the related code into a separate function? So that you'd
only have something like this in the realize function:

    for (i = 0, j = 0; i < bus_count; i++, j += MAX_IDE_DEVS) {
        if (!k->ide.is_enabled || k->ide.is_enabled(s, j)) {
            isa_superio_init_ide(...);
        }
    }

 Thomas



reply via email to

[Prev in Thread] Current Thread [Next in Thread]