Re: [PATCH] usb: includes: search and replace __attribute__ ((packed))

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 10, 2011 at 10:55:17AM -0800, Greg KH wrote:
> On Thu, Feb 10, 2011 at 02:00:58PM +0200, Felipe Balbi wrote:
> > checkpatch.pl prefers us to use __packed instead
> > of the more lengthy __attribute__ ((packed)), so
> > a simple search and replace fixed all occurrences
> > on all USB headers.
> > 
> > This patch was conceived with the following:
> > 
> > $ for i in $(git grep -e "__attribute__.((packed))"	\
> > 	include/linux/usb/ | cut -d: -f1 | uniq);	\
> > 	do sed -i 's/__attribute__ ((packed))/__packed/g' \
> > 	$i; done
> > 
> > It fixes checkpatch.pl warnings such as:
> > 
> >  WARNING: __packed is preferred over __attribute__((packed))
> >  #838: FILE: linux/usb/ch9.h:838:
> >  +} __attribute__((packed));
> > 
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> 
> Normally I'm all for checkpatch cleanups, but this is ridiculous.  Why
> is __packed better than "__attribute__ ((packed))"?  This seems way

because we have it ? :-)

> unnecessary to me, so I'll just ignore it for now if you don't mind.

You can ignore, for sure. But your question is the same as asking why do
we use 'inline' instead of '__inline' or '__inline__'. Or why we use
__init instead of the lengthy '__section(.init.text) __cold notrace' and
the answer to those are simple "because we have it" or "because it's
easier to type" or "because if we decide to change __packed to something
else, we change only on one place".

There are several reasons why we would prefer to use what the Kernel
gives us as infrastructure. But hey, it's your call. Below is the copy
of the commit which added that check to checkpatch.pl:

commit 3d130fd03e06672f7700e2cb694b29f9a98227ca
Author: Joe Perches <joe@xxxxxxxxxxx>
Date:   Wed Jan 12 17:00:00 2011 -0800

    checkpatch.pl: add "prefer __packed" check
    
    There's a __packed #define for __attribute__((packed)).  Add a checkpatch
    to tell people about it.
    
    Signed-off-by: Joe Perches <joe@xxxxxxxxxxx>
    Cc: Andy Whitcroft <apw@xxxxxxxxxxxx>
    Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fd9560e..4c0383d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2743,6 +2743,11 @@ sub process {
                        WARN("plain inline is preferred over $1\n" . $herecurr);
                }
 
+# Check for __attribute__ packed, prefer __packed
+               if ($line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) {
+                       WARN("__packed is preferred over __attribute__((packed))\n" . $herecurr);
+               }
+
 # check for sizeof(&)
                if ($line =~ /\bsizeof\s*\(\s*\&/) {
                        WARN("sizeof(& should be avoided\n" . $herecurr);

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux