diff mbox

[01/24] ath6kl: add bmi.c

Message ID 1310559554.1662.6.camel@Joe-Laptop (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Joe Perches July 13, 2011, 12:19 p.m. UTC 
On Wed, 2011-07-13 at 10:08 +0300, Kalle Valo wrote:
> On 07/13/2011 07:28 AM, Joe Perches wrote:
> > On Wed, 2011-07-13 at 04:33 +0300, Kalle Valo wrote: 
> >> diff --git a/drivers/net/wireless/ath/ath6kl/bmi.c b/drivers/net/wireless/ath/ath6kl/bmi.c
> > [] 
> >> +			ath6kl_err("Unable to decrement the command "
> >> +				   "credit count register: %d\n", ret);
> > I think all of these ath6kl_<level> uses in all the patches
> > should coalesce formats that are split across multiple lines.
> > This helps grepping the sources for a specific dmesg.
> > 			ath6kl_err("Unable to decrement the command credit count register: %d\n",
> > 				   ret);
> I agree, that would improve readibility. The reason why the lines were
> split is because of checkpatch warnings. I could unsplit them but then
> someone else might start complaining about high number checkpatch
> warnings about long lines. So I really don't know what to do with them.

Add this patch to checkpatch?
I'll submit it to Andrew Morton later.




--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Kalle Valo July 14, 2011, 6:46 a.m. UTC | #1 
On 07/13/2011 03:19 PM, Joe Perches wrote:
> On Wed, 2011-07-13 at 10:08 +0300, Kalle Valo wrote:
>> On 07/13/2011 07:28 AM, Joe Perches wrote:
>>> This helps grepping the sources for a specific dmesg.
>>> 			ath6kl_err("Unable to decrement the command credit count register: %d\n",
>>> 				   ret);
>> I agree, that would improve readibility. The reason why the lines were
>> split is because of checkpatch warnings. I could unsplit them but then
>> someone else might start complaining about high number checkpatch
>> warnings about long lines. So I really don't know what to do with them.
> 
> Add this patch to checkpatch?
> I'll submit it to Andrew Morton later.

Awesome! I had no idea checkpatch supports this. Just tested this with
ath6kl and I don't get long line warnings with long log messages.

Thank you very much for this!

I see that Andrew already took your patch but anyway:

Tested-by: Kalle Valo <kvalo@qca.qualcomm.comm>

Kalle
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches July 14, 2011, 7:33 a.m. UTC | #2 
On Thu, 2011-07-14 at 09:46 +0300, Kalle Valo wrote:
> On 07/13/2011 03:19 PM, Joe Perches wrote:
> > Add this patch to checkpatch?
> > I'll submit it to Andrew Morton later.
> Awesome! I had no idea checkpatch supports this. Just tested this with
> ath6kl and I don't get long line warnings with long log messages.

Note that this doesn't let you have arbitrarily
long lines just because it's a printk or any other
logging function.  Only the format portion of the
printk may exceed 80 columns.

Any additional arguments for the printk must not
start after or exceed 80 columns.

For instance below, the first ath6kl_info is fine,
the second one is not.

$ cat t.c
static int dump_obj(void)
{
	ath6kl_info("%s: 12345678901234567890123456789012345678901234567890123456789012345678901234567890\n",
		    "some long string");
	ath6kl_info("%s: 12345678901234567890123456789012345678901234567890123456789012345678901234567890\n", "some long string");
}

$ ./scripts/checkpatch.pl -f t.c
WARNING: line over 80 characters
#5: FILE: t.c:5:
+	ath6kl_info("%s: 12345678901234567890123456789012345678901234567890123456789012345678901234567890", "some long string");

total: 0 errors, 1 warnings, 6 lines checked

t.c has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo July 14, 2011, 7:38 a.m. UTC | #3 
On 07/14/2011 10:33 AM, Joe Perches wrote:
> On Thu, 2011-07-14 at 09:46 +0300, Kalle Valo wrote:
>> On 07/13/2011 03:19 PM, Joe Perches wrote:
>>> Add this patch to checkpatch?
>>> I'll submit it to Andrew Morton later.
>> Awesome! I had no idea checkpatch supports this. Just tested this with
>> ath6kl and I don't get long line warnings with long log messages.
> 
> Note that this doesn't let you have arbitrarily
> long lines just because it's a printk or any other
> logging function.  Only the format portion of the
> printk may exceed 80 columns.
> 
> Any additional arguments for the printk must not
> start after or exceed 80 columns.

Thanks, that's good to know. And it's a very nice feature. I also hate
long lines but splitting log messages doesn't really make any sense.

Kalle
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b0aa2c6..c0be3c5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -210,7 +210,7 @@  our $typeTypedefs = qr{(?x:
 
 our $logFunctions = qr{(?x:
 	printk|
-	[a-z]+_(emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)|
+	[a-z0-9]+_(emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)|
 	WARN|
 	panic|
 	MODULE_[A-Z_]+