| Message ID | 20201019203023.658555-1-ganqixin@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix line over 80 characters warning | expand |
On Tue, Oct 20, 2020 at 04:30:13AM +0800, Gan Qixin wrote: > Hi all, > I used scripts/checkpatch.pl to find that many files in the hw directory > contain lines with more than 80 characters. Therefore, I splited some lines to > fix this warning. Do we really need to still fix ourselves to a 80 col limit in the year 2020 ? Linux increased their max line length to 100 chars and even set checkpatch.pl to not complain about that limit unless --strict is given. 80 chars is fine as a "wish list" target, but IMHO the code often benefits more from exceeding 80 chars, and not wrapping. Regards, Daniel
On Tue, 20 Oct 2020 at 12:03, Gan Qixin <ganqixin@huawei.com> wrote: > > Hi all, > I used scripts/checkpatch.pl to find that many files in the hw directory > contain lines with more than 80 characters. Therefore, I splited some lines to > fix this warning. I personally have come round to the idea that we should instead adjust checkpatch so that it doesn't have a hard 80 character complaint limit. Compare the kernel coding style change: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144 whose rationale I agree with. We should *prefer* 80 character wrapping, but there are some places where an 85-character line is much more readable and sensible style than inserting a line break just to please checkpatch. thanks -- PMM
> -----Original Message----- > From: Peter Maydell [mailto:peter.maydell@linaro.org] > Sent: Tuesday, October 20, 2020 7:15 PM > To: ganqixin <ganqixin@huawei.com> > Cc: QEMU Developers <qemu-devel@nongnu.org>; QEMU Trivial > <qemu-trivial@nongnu.org>; Michael S. Tsirkin <mst@redhat.com>; Philippe > Mathieu-Daudé <f4bug@amsat.org>; Laurent Vivier <lvivier@redhat.com>; > David Gibson <david@gibson.dropbear.id.au>; Alistair Francis > <alistair.francis@wdc.com>; Chenqun (kuhn) <kuhn.chenqun@huawei.com>; > Zhanghailiang <zhang.zhanghailiang@huawei.com> > Subject: Re: [PATCH 00/10] Fix line over 80 characters warning > > On Tue, 20 Oct 2020 at 12:03, Gan Qixin <ganqixin@huawei.com> wrote: > > > > Hi all, > > I used scripts/checkpatch.pl to find that many files in the hw > > directory contain lines with more than 80 characters. Therefore, I > > splited some lines to fix this warning. > > I personally have come round to the idea that we should instead adjust > checkpatch so that it doesn't have a hard 80 character complaint limit. > Hi Peter, It sounds like a good idea, I think I can try to modify checkpatch.pl by referring to the Linux patch. Thanks, Gan Qixin > Compare the kernel coding style change: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id= > bdc48fa11e46f867ea4d75fa59ee87a7f48be144 > > whose rationale I agree with. We should *prefer* 80 character wrapping, but > there are some places where an 85-character line is much more readable and > sensible style than inserting a line break just to please checkpatch. > > thanks > -- PMM
> -----Original Message----- > From: Daniel P. Berrangé [mailto:berrange@redhat.com] > Sent: Tuesday, October 20, 2020 7:14 PM > To: ganqixin <ganqixin@huawei.com> > Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org; lvivier@redhat.com; > peter.maydell@linaro.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; > mst@redhat.com; f4bug@amsat.org; alistair.francis@wdc.com; Chenqun > (kuhn) <kuhn.chenqun@huawei.com>; david@gibson.dropbear.id.au > Subject: Re: [PATCH 00/10] Fix line over 80 characters warning > > On Tue, Oct 20, 2020 at 04:30:13AM +0800, Gan Qixin wrote: > > Hi all, > > I used scripts/checkpatch.pl to find that many files in the hw > > directory contain lines with more than 80 characters. Therefore, I > > splited some lines to fix this warning. > > Do we really need to still fix ourselves to a 80 col limit in the year 2020 ? > > Linux increased their max line length to 100 chars and even set checkpatch.pl > to not complain about that limit unless --strict is given. > > 80 chars is fine as a "wish list" target, but IMHO the code often benefits more > from exceeding 80 chars, and not wrapping. > Hi Daniel, Yes, you are right! I also found this problem when I try to fix these warning. In some cases, the 80-character limit doesn't necessarily make code more readable. Thanks, Gan Qixin > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- > https://www.instagram.com/dberrange :|
Hi all, I used scripts/checkpatch.pl to find that many files in the hw directory contain lines with more than 80 characters. Therefore, I splited some lines to fix this warning. Thanks, Gan Qixin Gan Qixin (10): hw/virtio/:split some lines containing more than 80 characters hw/core/:split some lines containing more than 80 characters hw/ide/:split some lines containing more than 80 characters hw/intc/:split some lines containing more than 80 characters hw/misc/:split some lines containing more than 80 characters hw/pci/:split some lines containing more than 80 characters hw/pci-host/:split some lines containing more than 80 characters hw/char/:split some lines containing more than 80 characters hw/input/:split some lines containing more than 80 characters hw/riscv/:split some lines containing more than 80 characters hw/char/ibex_uart.c | 12 ++++--- hw/char/omap_uart.c | 3 +- hw/char/parallel.c | 12 ++++--- hw/char/serial.c | 57 ++++++++++++++++++++++---------- hw/char/virtio-serial-bus.c | 3 +- hw/core/bus.c | 3 +- hw/core/loader.c | 17 ++++++---- hw/core/machine-hmp-cmds.c | 6 ++-- hw/core/machine.c | 3 +- hw/core/qdev-properties-system.c | 4 +-- hw/ide/ahci.c | 10 +++--- hw/ide/atapi.c | 9 ++--- hw/ide/cmd646.c | 3 +- hw/ide/core.c | 21 ++++++++---- hw/ide/piix.c | 3 +- hw/ide/via.c | 3 +- hw/input/hid.c | 3 +- hw/input/milkymist-softusb.c | 16 +++++---- hw/input/pxa2xx_keypad.c | 3 +- hw/input/virtio-input.c | 3 +- hw/intc/apic.c | 3 +- hw/intc/arm_gic.c | 5 +-- hw/intc/arm_gic_common.c | 3 +- hw/intc/ioapic.c | 3 +- hw/intc/xics.c | 3 +- hw/intc/xics_kvm.c | 3 +- hw/misc/aspeed_sdmc.c | 10 +++--- hw/misc/bcm2835_mphi.c | 3 +- hw/misc/edu.c | 3 +- hw/misc/omap_gpmc.c | 3 +- hw/misc/omap_sdrc.c | 3 +- hw/misc/pci-testdev.c | 3 +- hw/misc/sifive_test.c | 4 +-- hw/pci-host/gpex-acpi.c | 18 +++++----- hw/pci-host/pam.c | 4 +-- hw/pci-host/ppce500.c | 8 +++-- hw/pci-host/q35.c | 11 +++--- hw/pci-host/versatile.c | 5 +-- hw/pci/msi.c | 3 +- hw/pci/msix.c | 8 ++--- hw/pci/pci.c | 31 +++++++++++------ hw/pci/pci_bridge.c | 3 +- hw/pci/pcie.c | 11 +++--- hw/pci/pcie_host.c | 4 +-- hw/riscv/opentitan.c | 6 ++-- hw/riscv/sifive_e.c | 6 ++-- hw/riscv/sifive_u.c | 12 ++++--- hw/virtio/vhost-backend.c | 3 +- hw/virtio/vhost-user-fs.c | 6 ++-- hw/virtio/vhost-user.c | 10 +++--- hw/virtio/virtio-balloon.c | 6 ++-- hw/virtio/virtio-bus.c | 3 +- hw/virtio/virtio-crypto.c | 3 +- hw/virtio/virtio-pci.c | 4 +-- hw/virtio/virtio-pci.h | 8 +++-- hw/virtio/virtio-rng.c | 3 +- hw/virtio/virtio.c | 14 +++++--- 57 files changed, 273 insertions(+), 160 deletions(-)