[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: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH for-4.1 1/4] hw/isa/superio: Rename a variable |
Date: |
Fri, 5 Apr 2019 11:32:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 4/5/19 6:14 AM, Thomas Huth wrote:
> 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.
Me too ;) But sometimes I find the 80 chars/line limit generate
indentation harder to review. But here I have no excuse :)
> 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(...);
> }
> }
Yes, fair enough. Note you used the shortened variables in your example! :P
Regards,
Phil.