Linux ATA/IDE development
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Mark Lord <kernel@teksavvy.com>
Cc: linux-kernel@vger.kernel.org, Jeff Garzik <jgarzik@pobox.com>,
	linux-ide@vger.kernel.org
Subject: Re: [PATCH 02/10] drivers/ata: Fix continuation line formats
Date: Wed, 03 Feb 2010 11:15:35 -0800	[thread overview]
Message-ID: <1265224535.24887.16.camel@Joe-Laptop.home> (raw)
In-Reply-To: <4B69B0A0.2040701@teksavvy.com>

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;




  parent reply	other threads:[~2010-02-03 19:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-02  7:22 [PATCH 00/10] Non-printk continuation line fixes Joe Perches
2010-02-02  7:22 ` [PATCH 02/10] drivers/ata: Fix continuation line formats Joe Perches
2010-02-03 17:21   ` Mark Lord
2010-02-03 17:25     ` Jeff Garzik
2010-02-07 15:10       ` Sergey Matyukevich
2010-02-03 19:15     ` Joe Perches [this message]
2010-02-04  1:39       ` Krzysztof Halasa
2010-02-04  3:36         ` Joe Perches
2010-02-04  4:59       ` Mark Lord
2010-02-04  5:13         ` Andrew Morton
2010-02-04 14:03           ` Mark Lord
2010-02-04  5:16         ` Joe Perches
2010-02-04 14:07           ` Mark Lord

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1265224535.24887.16.camel@Joe-Laptop.home \
    --to=joe@perches.com \
    --cc=jgarzik@pobox.com \
    --cc=kernel@teksavvy.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox