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é <phi...@redhat.com> >> --- >> 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.