linux-kernel - Re: [PATCH 02/10] drivers/ata: Fix continuation line formats

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1265224535.24887.16.camel@Joe-Laptop.home>
Date:	Wed, 03 Feb 2010 11:15:35 -0800
From:	Joe Perches <joe@...ches.com>
To:	Mark Lord <kernel@...savvy.com>
Cc:	linux-kernel@...r.kernel.org, Jeff Garzik <jgarzik@...ox.com>,
	linux-ide@...r.kernel.org
Subject: Re: [PATCH 02/10] drivers/ata: Fix continuation line formats

On Wed, 2010-02-03 at 12:21 -0500, Mark Lord wrote:
> Joe Perches wrote:
> > String constants that are continued on subsequent lines with \
> > are not good.
[]
> > -		dev_warn(ap->dev, "Failed to compute ATA timing %d, \
> > -				set PIO_0 timing\n", ret);
> > +		dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
> > +			 ret);
> NAK.  Just stick the ret); part onto the same line and be done with it.

:resend that I mistakenly sent only to Mark, with a couple more comments

I disagree.

Look at the long line wrapping style of the rest of the file
and most of the kernel source.  No additional arguments to
functions are generally used after column 80.

If it's ever agreed that all lines > 80 cols are OK or
new args after column 80 are OK, then sure.  Until then,
if you want it, you should do it.

cheers, Joe

$ grep -A 1 "\bdev_" drivers/ata/pata_at91.c 
	dev_dbg(dev, "ATA timings: nrd_setup = %lu nrd_pulse = %lu nrd_cycle = %lu\n",
			nrd_setup, nrd_pulse, read_cycle);
	dev_dbg(dev, "ATA timings: nwe_setup = %lu nwe_pulse = %lu nwe_cycle = %lu\n",
			nwe_setup, nwe_pulse, write_cycle);
	dev_dbg(dev, "ATA timings: ncs_read_setup = %lu ncs_read_pulse = %lu\n",
			ncs_read_setup, ncs_read_pulse);
	dev_dbg(dev, "ATA timings: ncs_write_setup = %lu ncs_write_pulse = %lu\n",
			ncs_write_setup, ncs_write_pulse);
--
		dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
			 ret);
--
		dev_err(&pdev->dev, "invalid number of resources\n");
		return -EINVAL;
--
		dev_err(dev, "failed to get mem resource\n");
		return -EINVAL;
--
		dev_err(dev, "failed to allocate memory for private data\n");
		return -ENOMEM;
--
		dev_err(dev, "failed to get access to mck clock\n");
		return -ENODEV;
--
		dev_err(dev, "failed to map IO base\n");
		ret = -ENOMEM;
--
		dev_err(dev, "failed to map CTL base\n");
		ret = -ENOMEM;
--
	struct ata_host *host = dev_get_drvdata(&pdev->dev);
	struct at91_ide_info *info;



--
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/