| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 24 May 2009 16:12:11 +0400
From: Sergei Shtylyov <sshtylyov@...mvista.com>
To: Joe Perches <joe@...ches.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
Borislav Petkov <petkovbb@...il.com>,
Tejun Heo <tj@...nel.org>, linux-ide@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] drivers/ide/ide-core: Unsplit constant strings for
pr_<level> and dev_<level>
Hello.
Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@...ches.com>
>
[...]
> diff --git a/drivers/ide/ide-acpi.c b/drivers/ide/ide-acpi.c
> index f323a60..560b9c4 100644
> --- a/drivers/ide/ide-acpi.c
> +++ b/drivers/ide/ide-acpi.c
> @@ -435,8 +435,8 @@ void ide_acpi_get_timing(ide_hwif_t *hwif)
> if (!out_obj->buffer.length || !out_obj->buffer.pointer ||
> out_obj->buffer.length != sizeof(struct GTM_buffer)) {
> kfree(output.pointer);
> - pr_err("%s: unexpected _GTM length (0x%x)[should be 0x%zx] or "
> - "addr (0x%p)\n",
> + pr_err(
> + "%s: unexpected _GTM length (0x%x)[should be 0x%zx] or addr (0x%p)\n",
>
Oh no... do you really think somebody will search for this message
using strings like "should be"?
> __func__, out_obj->buffer.length,
> sizeof(struct GTM_buffer), out_obj->buffer.pointer);
> return;
> diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
> index 46cf46e..1e9e2d6 100644
> --- a/drivers/ide/ide-atapi.c
> +++ b/drivers/ide/ide-atapi.c
> @@ -60,11 +60,13 @@ int ide_check_atapi_device(ide_drive_t *drive, const char *s)
> dev_err(&drive->gendev, "%s: the removable flag is not set\n",
> s);
> else if (drive->media == ide_floppy && drq_type == 3)
> - dev_err(&drive->gendev, "%s: sorry, DRQ type (0x%02x) not "
> - "supported\n", s, drq_type);
> + dev_err(&drive->gendev,
> + "%s: sorry, DRQ type (0x%02x) not supported\n",
>
Same question. I think the user will search for "sorry, DRQ type".
> + s, drq_type);
> else if (packet_size != 0)
> - dev_err(&drive->gendev, "%s: packet size (0x%02x) is not 12 "
> - "bytes\n", s, packet_size);
> + dev_err(&drive->gendev,
> + "%s: packet size (0x%02x) is not 12 bytes\n",
>
When the message is broken by the format specifier, turning it into
one liner can hardly help seraching...
> @@ -439,8 +440,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
> if (pc->flags & PC_FLAG_DMA_IN_PROGRESS) {
> pc->flags &= ~PC_FLAG_DMA_IN_PROGRESS;
> dev_err(&drive->gendev,
> - "The device wants to issue more interrupts "
> - "in DMA mode\n");
> + "The device wants to issue more interrupts in DMA mode\n");
>
Oh noes, the indentation...
> @@ -509,14 +509,12 @@ static u8 ide_wait_ireason(ide_drive_t *drive, u8 ireason)
> while (retries-- && ((ireason & ATAPI_COD) == 0 ||
> (ireason & ATAPI_IO))) {
> dev_err(&drive->gendev,
> - "(IO,CoD != (0,1) while issuing "
> - "a packet command, retrying\n");
> + "(IO,CoD != (0,1) while issuing a packet command, retrying\n");
>
Sigh...
> udelay(100);
> ireason = ide_read_ireason(drive);
> if (retries == 0) {
> dev_err(&drive->gendev,
> - "(IO,CoD != (0,1) while issuing "
> - "a packet command, ignoring\n");
> + "(IO,CoD != (0,1) while issuing a packet command, ignoring\n");
>
Sigh...
> @@ -547,8 +545,7 @@ static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
>
> if (ide_wait_stat(&startstop, drive, ATA_DRQ, ATA_BUSY, WAIT_READY)) {
> dev_err(&drive->gendev,
> - "Strange, packet command initiated yet "
> - "DRQ isn't asserted\n");
> + "Strange, packet command initiated yet DRQ isn't asserted\n");
>
Sigh...
> @@ -590,8 +587,7 @@ static ide_startstop_t ide_transfer_pc(ide_drive_t *drive)
>
> if ((ireason & ATAPI_COD) == 0 || (ireason & ATAPI_IO)) {
> dev_err(&drive->gendev,
> - "(IO,CoD) != (0,1) while issuing "
> - "a packet command\n");
> + "(IO,CoD) != (0,1) while issuing a packet command\n");
>
Sigh...
> diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
> index 51c7cb4..07565c9 100644
> --- a/drivers/ide/ide-io.c
> +++ b/drivers/ide/ide-io.c
> @@ -743,8 +743,9 @@ static void unexpected_intr(int irq, ide_hwif_t *hwif)
>
> if (time_after(jiffies, last_msgtime + HZ)) {
> last_msgtime = jiffies;
> - dev_err(&hwif->gendev, "unexpected interrupt, "
> - "status=0x%02x, count=%ld\n", stat, count);
> + dev_err(&hwif->gendev,
> + "unexpected interrupt, status=0x%02x, count=%ld\n",
>
Hardly won anything...
> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
> index 6dbb96e..c5ab2ec 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -259,8 +259,8 @@ no_80w:
> if (drive->dev_flags & IDE_DFLAG_UDMA33_WARNED)
> return 0;
>
> - dev_warn(&drive->gendev, "%s side 80-wire cable detection failed, "
> - "limiting max speed to UDMA33\n",
> + dev_warn(&drive->gendev,
> + "%s side 80-wire cable detection failed, limiting max speed to UDMA33\n",
>
Will you really use the full message to serch here?
> diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
> index c4b44e2..a4120e2 100644
> --- a/drivers/ide/ide-probe.c
> +++ b/drivers/ide/ide-probe.c
> @@ -271,8 +271,9 @@ int ide_dev_read_id(ide_drive_t *drive, u8 cmd, u16 *id)
> s = tp_ops->read_status(hwif);
> if ((a ^ s) & ~ATA_IDX)
> /* ancient Seagate drives, broken interfaces */
> - dev_info(&drive->gendev, "probing with STATUS(0x%02x) "
> - "instead of ALTSTATUS(0x%02x)\n", s, a);
> + dev_info(&drive->gendev,
> + "probing with STATUS(0x%02x) instead of ALTSTATUS(0x%02x)\n",
>
Same comment about the line broken by the format specifiers. This
wins absolutely nothing.
> @@ -415,8 +416,8 @@ static int do_probe (ide_drive_t *drive, u8 cmd)
>
> if (rc == 1 && cmd == ATA_CMD_ID_ATAPI) {
> dev_err(&drive->gendev,
> - "no response (status = 0x%02x), "
> - "resetting drive\n", stat);
> + "no response (status = 0x%02x), resetting drive\n",
>
And again...
> diff --git a/drivers/ide/ide-proc.c b/drivers/ide/ide-proc.c
> index 23b9d6a..cd580b1 100644
> --- a/drivers/ide/ide-proc.c
> +++ b/drivers/ide/ide-proc.c
> @@ -464,8 +464,7 @@ static int ide_replace_subdriver(ide_drive_t *drive, const char *driver)
> strlcpy(drive->driver_req, driver, sizeof(drive->driver_req));
> err = device_attach(dev);
> if (err < 0)
> - pr_warning("IDE: %s: device_attach error: %d\n",
> - __func__, err);
> + pr_warning("IDE: %s: device_attach error: %d\n", __func__, err);
>
I'm nt sure why this isn't a part of printk(KERN_*, ...); to
pr_*(...) conversion patch. You're not unsplitting the constant here.
> diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
> index 8d741d1..687b3be 100644
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -579,8 +579,9 @@ int ide_taskfile_ioctl(ide_drive_t *drive, unsigned long arg)
> case TASKFILE_MULTI_OUT:
> if (!drive->mult_count) {
> /* (hs): give up if multcount is not set */
> - dev_err(&drive->gendev, "%s Multimode Write " \
> - "multcount is not set\n", __func__);
> + dev_err(&drive->gendev,
> + "%s Multimode Write multcount is not set\n",
>
Sigh...
> @@ -599,8 +600,8 @@ int ide_taskfile_ioctl(ide_drive_t *drive, unsigned long arg)
> if (!drive->mult_count) {
> /* (hs): give up if multcount is not set */
> dev_err(&drive->gendev,
> - "%s Multimode Read failure "
> - "multcount is not set\n", __func__);
> + "%s Multimode Read failure multcount is not set\n",
> + __func__);
>
Sigh...
> diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> index f139cf3..3aca89a 100644
> --- a/drivers/ide/setup-pci.c
> +++ b/drivers/ide/setup-pci.c
> @@ -39,17 +39,18 @@ static int ide_setup_pci_baseregs(struct pci_dev *dev, const char *name)
> if (pci_read_config_byte(dev, PCI_CLASS_PROG, &progif) ||
> (progif & 5) != 5) {
> if ((progif & 0xa) != 0xa) {
> - pr_info("%s %s: device not capable of full "
> - "native PCI mode\n", name, pci_name(dev));
> + pr_info(
> + "%s %s: device not capable of full native PCI mode\n",
>
Sigh...
> (void) pci_write_config_byte(dev, PCI_CLASS_PROG, progif|5);
> if (pci_read_config_byte(dev, PCI_CLASS_PROG, &progif) ||
> (progif & 5) != 5) {
> - pr_err("%s %s: rewrite of PROGIF failed, "
> - "wanted 0x%04x, got 0x%04x\n",
> + pr_err(
> + "%s %s: rewrite of PROGIF failed, wanted 0x%04x, got 0x%04x\n",
>
The same comment about the string broken by yhe format specifiers.
This wins nothing, well, almost...
> @@ -322,8 +323,8 @@ static int ide_hw_configure(struct pci_dev *dev, const struct ide_port_info *d,
> if ((d->host_flags & IDE_HFLAG_ISA_PORTS) == 0) {
> if (ide_pci_check_iomem(dev, d, 2 * port) ||
> ide_pci_check_iomem(dev, d, 2 * port + 1)) {
> - pr_err("%s %s: I/O baseregs (BIOS) are "
> - "reported as MEM for port %d!\n",
> + pr_err(
> + "%s %s: I/O baseregs (BIOS) are reported as MEM for port %d!\n",
>
Sigh...
> @@ -519,8 +520,9 @@ static int do_ide_setup_pci_device(struct pci_dev *dev,
>
> if (ide_pci_is_in_compatibility_mode(dev)) {
> if (noisy)
> - pr_info("%s %s: not 100%% native mode: will "
> - "probe irqs later\n", d->name, pci_name(dev));
> + pr_info(
> + "%s %s: not 100%% native mode: will probe irqs later\n",
>
Sigh...
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists