* [PATCH] Bluetooth: Fix coding style @ 2012-05-04 18:59 Gustavo Padovan 2012-05-06 16:02 ` Marcel Holtmann 2012-05-06 16:36 ` David Miller 0 siblings, 2 replies; 22+ messages in thread From: Gustavo Padovan @ 2012-05-04 18:59 UTC (permalink / raw) To: linville; +Cc: linux-bluetooth, linux-wireless Fix offending styles all over the tree. This is in conformance David Miller's style rules. Signed-off-by: Gustavo Padovan <gustavo@padovan.org> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_event.c | 2 +- net/bluetooth/l2cap_core.c | 12 ++++++------ net/bluetooth/l2cap_sock.c | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index ef6e654..6148352 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -433,7 +433,7 @@ static inline bool hci_conn_ssp_enabled(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; return (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags) && - test_bit(HCI_CONN_SSP_ENABLED, &conn->flags)); + test_bit(HCI_CONN_SSP_ENABLED, &conn->flags)); } static inline void hci_conn_hash_init(struct hci_dev *hdev) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index fb23c47..a094315 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1122,7 +1122,7 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev, schedule_delayed_work(&hdev->adv_work, ADV_CLEAR_TIMEOUT); if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED && - hdev->discovery.state == DISCOVERY_FINDING) { + hdev->discovery.state == DISCOVERY_FINDING) { mgmt_interleaved_discovery(hdev); } else { hci_dev_lock(hdev); diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 9d6c650..89a50ed 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -426,7 +426,7 @@ void l2cap_chan_set_defaults(struct l2cap_chan *chan) static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) { BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn, - __le16_to_cpu(chan->psm), chan->dcid); + __le16_to_cpu(chan->psm), chan->dcid); conn->disc_reason = HCI_ERROR_REMOTE_USER_TERM; @@ -1128,7 +1128,7 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid, src_any = !bacmp(&bt_sk(sk)->src, BDADDR_ANY); dst_any = !bacmp(&bt_sk(sk)->dst, BDADDR_ANY); if ((src_match && dst_any) || (src_any && dst_match) || - (src_any && dst_any)) + (src_any && dst_any)) c1 = c; } } @@ -1385,7 +1385,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, src_any = !bacmp(&bt_sk(sk)->src, BDADDR_ANY); dst_any = !bacmp(&bt_sk(sk)->dst, BDADDR_ANY); if ((src_match && dst_any) || (src_any && dst_match) || - (src_any && dst_any)) + (src_any && dst_any)) c1 = c; } } @@ -1406,7 +1406,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *d int err; BT_DBG("%s -> %s psm 0x%2.2x", batostr(src), batostr(dst), - __le16_to_cpu(chan->psm)); + __le16_to_cpu(chan->psm)); hdev = hci_get_route(dst, src); if (!hdev) @@ -3245,8 +3245,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr flags = __le16_to_cpu(rsp->flags); result = __le16_to_cpu(rsp->result); - BT_DBG("scid 0x%4.4x flags 0x%2.2x result 0x%2.2x len %d", - scid, flags, result, len); + BT_DBG("scid 0x%4.4x flags 0x%2.2x result 0x%2.2x len %d", scid, flags, + result, len); chan = l2cap_get_chan_by_scid(conn, scid); if (!chan) diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 4d03b45..8d8b50a 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -938,7 +938,7 @@ static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan, skb = bt_skb_send_alloc(chan->sk, len, nb, &err); if (!skb) - return (ERR_PTR(err)); + return ERR_PTR(err); return skb; } -- 1.7.10 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-04 18:59 [PATCH] Bluetooth: Fix coding style Gustavo Padovan @ 2012-05-06 16:02 ` Marcel Holtmann 2012-05-06 16:36 ` David Miller 1 sibling, 0 replies; 22+ messages in thread From: Marcel Holtmann @ 2012-05-06 16:02 UTC (permalink / raw) To: Gustavo Padovan; +Cc: linville, linux-bluetooth, linux-wireless Hi Gustavo, > Fix offending styles all over the tree. This is in conformance David > Miller's style rules. > > Signed-off-by: Gustavo Padovan <gustavo@padovan.org> > --- > include/net/bluetooth/hci_core.h | 2 +- > net/bluetooth/hci_event.c | 2 +- > net/bluetooth/l2cap_core.c | 12 ++++++------ > net/bluetooth/l2cap_sock.c | 2 +- > 4 files changed, 9 insertions(+), 9 deletions(-) Acked-by: Marcel Holtmann <marcel@holtmann.org> Regards Marcel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-04 18:59 [PATCH] Bluetooth: Fix coding style Gustavo Padovan 2012-05-06 16:02 ` Marcel Holtmann @ 2012-05-06 16:36 ` David Miller 2012-05-06 17:46 ` David Herrmann 2012-05-07 15:24 ` Gustavo Padovan 1 sibling, 2 replies; 22+ messages in thread From: David Miller @ 2012-05-06 16:36 UTC (permalink / raw) To: gustavo; +Cc: linville, linux-bluetooth, linux-wireless From: Gustavo Padovan <gustavo@padovan.org> Date: Fri, 4 May 2012 15:59:48 -0300 > Fix offending styles all over the tree. This is in conformance David > Miller's style rules. They aren't just my style rules. And, if your plan is to patch things up afterwards so you don't have to change existing commits in your tree, think again. I absolutely will not pull a tree from John that has commits in it that add the bad indentation. You have to respin your tree with fixed commits. The bluetooth folks have to understand that they are on an extremely short leash right now. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-06 16:36 ` David Miller @ 2012-05-06 17:46 ` David Herrmann 2012-05-06 18:53 ` David Miller 2012-05-07 15:24 ` Gustavo Padovan 1 sibling, 1 reply; 22+ messages in thread From: David Herrmann @ 2012-05-06 17:46 UTC (permalink / raw) To: David Miller; +Cc: gustavo, linville, linux-bluetooth, linux-wireless Hi David On Sun, May 6, 2012 at 6:36 PM, David Miller <davem@davemloft.net> wrote: > From: Gustavo Padovan <gustavo@padovan.org> > Date: Fri, 4 May 2012 15:59:48 -0300 > >> Fix offending styles all over the tree. This is in conformance David >> Miller's style rules. > > They aren't just my style rules. Whose rules are they? Reading ./Documentation/CodingStyle I see: +++++++ Outside of comments, documentation and except in Kconfig, spaces are never used for indentation, and the above example is deliberately broken. +++++++ Or looking at the last 3 of Linus' commits that use multi-line expressions, none of them uses mixed tabs+spaces to have lined-up indentation (which is what you recommend): e419b4cc585680940bc42f8ca8a071d6023fb1bb vfs: make word-at-a-time... 6be5ceb02e98eaf6cfc4f8b12a896d04023f340d VM: Add "vm_mmap" helper... a554bea89948dfb6d2f9c4c62ce2b12b2dac18ad selinux: don't inline... So why so reluctant to calling it "David's style"? > And, if your plan is to patch things up afterwards so you don't > have to change existing commits in your tree, think again. > > I absolutely will not pull a tree from John that has commits in > it that add the bad indentation. > > You have to respin your tree with fixed commits. That's just old code that is reindented. Nothing special. All new commits use the new coding-style properly. > The bluetooth folks have to understand that they are on an extremely > short leash right now. Regards David ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-06 17:46 ` David Herrmann @ 2012-05-06 18:53 ` David Miller 2012-05-07 8:14 ` Andrei Emeltchenko ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: David Miller @ 2012-05-06 18:53 UTC (permalink / raw) To: dh.herrmann; +Cc: gustavo, linville, linux-bluetooth, linux-wireless From: David Herrmann <dh.herrmann@googlemail.com> Date: Sun, 6 May 2012 19:46:46 +0200 > Whose rules are they? Find me an example in another major core subsystem, let's use mm/memory.c as an example as that file gets hit by a lot of people, that uses the multi-line conditional TAB-only crap you guys seem to keep using. They don't. All the examples you'll find are of the form: if (a && b) not: if (a && b) like I see happening in bluetooth all the time. How did you think they figured that out? Did they read someone's mind? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-06 18:53 ` David Miller @ 2012-05-07 8:14 ` Andrei Emeltchenko 2012-05-07 8:21 ` Arend van Spriel 2012-05-07 15:52 ` David Miller 2012-05-07 17:06 ` Marcel Holtmann 2012-05-07 19:40 ` Lucas De Marchi 2 siblings, 2 replies; 22+ messages in thread From: Andrei Emeltchenko @ 2012-05-07 8:14 UTC (permalink / raw) To: David Miller Cc: dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless Hi David, On Sun, May 06, 2012 at 02:53:04PM -0400, David Miller wrote: > From: David Herrmann <dh.herrmann@googlemail.com> > Date: Sun, 6 May 2012 19:46:46 +0200 > > > Whose rules are they? > > Find me an example in another major core subsystem, let's use > mm/memory.c as an example as that file gets hit by a lot of people, > that uses the multi-line conditional TAB-only crap you guys seem to > keep using. If you think that Documentation/CodingStyle is a crap why don't you send a patch to fix it? > They don't. All the examples you'll find are of the form: > > if (a && > b) > > not: > > if (a && > b) Actually it does not look like this, otherwise "b" would be placed in the same line, don't it? Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 8:14 ` Andrei Emeltchenko @ 2012-05-07 8:21 ` Arend van Spriel 2012-05-07 8:29 ` Andrei Emeltchenko 2012-05-07 15:52 ` David Miller 1 sibling, 1 reply; 22+ messages in thread From: Arend van Spriel @ 2012-05-07 8:21 UTC (permalink / raw) To: Andrei Emeltchenko, David Miller, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless On 05/07/2012 10:14 AM, Andrei Emeltchenko wrote: >> They don't. All the examples you'll find are of the form: >> > >> > if (a && >> > b) >> > >> > not: >> > >> > if (a && >> > b) > Actually it does not look like this, otherwise "b" would be placed in the > same line, don't it? Let me rephrase Dave's remark so you may understand: if (a_has_to_be_long_enough_to_make_you_understand && you_do_not_care_about_code_readability) and not: if (a_has_to_be_long_enough_to_make_you_understand && you_do_not_care_about_code_readability) Hope it helps. Gr. AvS ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 8:21 ` Arend van Spriel @ 2012-05-07 8:29 ` Andrei Emeltchenko 2012-05-07 10:06 ` Arend van Spriel 0 siblings, 1 reply; 22+ messages in thread From: Andrei Emeltchenko @ 2012-05-07 8:29 UTC (permalink / raw) To: Arend van Spriel Cc: David Miller, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless Hi Arend, On Mon, May 07, 2012 at 10:21:44AM +0200, Arend van Spriel wrote: > On 05/07/2012 10:14 AM, Andrei Emeltchenko wrote: > >> They don't. All the examples you'll find are of the form: > >> > > >> > if (a && > >> > b) > >> > > >> > not: > >> > > >> > if (a && > >> > b) > > Actually it does not look like this, otherwise "b" would be placed in the > > same line, don't it? > > Let me rephrase Dave's remark so you may understand: > > if (a_has_to_be_long_enough_to_make_you_understand && > you_do_not_care_about_code_readability) > > and not: > if (a_has_to_be_long_enough_to_make_you_understand && > you_do_not_care_about_code_readability) You seems to forgot to reply to the main question about Codying Style. Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 8:29 ` Andrei Emeltchenko @ 2012-05-07 10:06 ` Arend van Spriel 0 siblings, 0 replies; 22+ messages in thread From: Arend van Spriel @ 2012-05-07 10:06 UTC (permalink / raw) To: Andrei Emeltchenko, David Miller, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless On 05/07/2012 10:29 AM, Andrei Emeltchenko wrote: > Hi Arend, > > On Mon, May 07, 2012 at 10:21:44AM +0200, Arend van Spriel wrote: >> On 05/07/2012 10:14 AM, Andrei Emeltchenko wrote: >>>> They don't. All the examples you'll find are of the form: >>>>> >>>>> if (a && >>>>> b) >>>>> >>>>> not: >>>>> >>>>> if (a && >>>>> b) >>> Actually it does not look like this, otherwise "b" would be placed in the >>> same line, don't it? >> >> Let me rephrase Dave's remark so you may understand: >> >> if (a_has_to_be_long_enough_to_make_you_understand && >> you_do_not_care_about_code_readability) >> >> and not: >> if (a_has_to_be_long_enough_to_make_you_understand && >> you_do_not_care_about_code_readability) > > You seems to forgot to reply to the main question about Codying Style. > > Best regards > Andrei Emeltchenko True. But you explicitly addressed that one to David. The alignment rule is indeed not stated in CodingStyle document, but using a seemingly random amount of tabs is obviously not improving readability. I suppose the alignment rule with an example could be added to the CodingStyle. Gr. AvS ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 8:14 ` Andrei Emeltchenko 2012-05-07 8:21 ` Arend van Spriel @ 2012-05-07 15:52 ` David Miller 2012-05-07 19:49 ` Lucas De Marchi 1 sibling, 1 reply; 22+ messages in thread From: David Miller @ 2012-05-07 15:52 UTC (permalink / raw) To: andrei.emeltchenko.news Cc: dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless From: Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> Date: Mon, 7 May 2012 11:14:13 +0300 > If you think that Documentation/CodingStyle is a crap why don't you send a > patch to fix it? The same reason I don't go try and fix broken networking RFCs. Documents are never are the final verdict on anything, there will always be areas where such documents are wrong and common sense should prevail. If you take them %100 of the time on face value, and use them as an opportunity to turn your brain completely off, that's your problem. And it's amusing that you ask me to fix the document. You're unwillingness to go look at other core areas of the kernel and see what they do there is just an evasive act rather than an attempt to understand the point I'm trying to make. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 15:52 ` David Miller @ 2012-05-07 19:49 ` Lucas De Marchi 2012-05-07 20:06 ` David Miller 0 siblings, 1 reply; 22+ messages in thread From: Lucas De Marchi @ 2012-05-07 19:49 UTC (permalink / raw) To: David Miller Cc: andrei.emeltchenko.news, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless On Mon, May 7, 2012 at 12:52 PM, David Miller <davem@davemloft.net> wrote: > From: Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> > Date: Mon, 7 May 2012 11:14:13 +0300 > >> If you think that Documentation/CodingStyle is a crap why don't you send a >> patch to fix it? > > The same reason I don't go try and fix broken networking RFCs. > > Documents are never are the final verdict on anything, there will > always be areas where such documents are wrong and common sense should > prevail. Then we go and fix the docs. This is how they work, or at least should. Doing so also means we don't create our own rules and impose them on others, like happened some time ago (https://lkml.org/lkml/2012/4/16/473). Regards, Lucas De Marchi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 19:49 ` Lucas De Marchi @ 2012-05-07 20:06 ` David Miller 2012-05-07 20:14 ` Arend van Spriel 2012-05-07 21:33 ` Marcel Holtmann 0 siblings, 2 replies; 22+ messages in thread From: David Miller @ 2012-05-07 20:06 UTC (permalink / raw) To: lucas.demarchi Cc: andrei.emeltchenko.news, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless From: Lucas De Marchi <lucas.demarchi@profusion.mobi> Date: Mon, 7 May 2012 16:49:16 -0300 > Doing so also means we don't create our own rules and impose them on > others, like happened some time ago > (https://lkml.org/lkml/2012/4/16/473). You conveniently forgot to show that Linus said that it's OK for a subsystem maintainer to request that kind of coding style, it's just not OK to impose it tree-wide. Any particular reason you guys make such a big deal about this whereas everyone else is quite fine with it and never gives me any problems? In the end, this hurts bluetooth, because if you anger me that means it's going to be harder and take longer for your changes to get into the tree. You can ignore this pracitcal aspect of this whole ordeal at your own peril. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 20:06 ` David Miller @ 2012-05-07 20:14 ` Arend van Spriel 2012-05-07 21:33 ` Marcel Holtmann 1 sibling, 0 replies; 22+ messages in thread From: Arend van Spriel @ 2012-05-07 20:14 UTC (permalink / raw) To: David Miller Cc: lucas.demarchi, andrei.emeltchenko.news, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless On 05/07/2012 10:06 PM, David Miller wrote: > From: Lucas De Marchi <lucas.demarchi@profusion.mobi> > Date: Mon, 7 May 2012 16:49:16 -0300 > >> Doing so also means we don't create our own rules and impose them on >> others, like happened some time ago >> (https://lkml.org/lkml/2012/4/16/473). > > You conveniently forgot to show that Linus said that it's OK for a > subsystem maintainer to request that kind of coding style, it's just > not OK to impose it tree-wide. Actually, checkpatch.pl does validate the rule being discussed here. That could be regarded as imposing it. The SubmittingPatches document states the following: "At a minimum you should check your patches with the patch style checker prior to submission (scripts/checkpatch.pl). You should be able to justify all violations that remain in your patch." Gr. AvS ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 20:06 ` David Miller 2012-05-07 20:14 ` Arend van Spriel @ 2012-05-07 21:33 ` Marcel Holtmann 2012-05-07 21:41 ` David Miller 1 sibling, 1 reply; 22+ messages in thread From: Marcel Holtmann @ 2012-05-07 21:33 UTC (permalink / raw) To: David Miller Cc: lucas.demarchi, andrei.emeltchenko.news, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless Hi Dave, > > Doing so also means we don't create our own rules and impose them on > > others, like happened some time ago > > (https://lkml.org/lkml/2012/4/16/473). > > You conveniently forgot to show that Linus said that it's OK for a > subsystem maintainer to request that kind of coding style, it's just > not OK to impose it tree-wide. may I ask some clarification on to what level of subsystem maintainers this applies. Is this only 1st level subsystem. Or would this extend to 2nd level subsystems as well? Regards Marcel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 21:33 ` Marcel Holtmann @ 2012-05-07 21:41 ` David Miller 0 siblings, 0 replies; 22+ messages in thread From: David Miller @ 2012-05-07 21:41 UTC (permalink / raw) To: marcel Cc: lucas.demarchi, andrei.emeltchenko.news, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless From: Marcel Holtmann <marcel@holtmann.org> Date: Mon, 07 May 2012 14:33:39 -0700 >> > Doing so also means we don't create our own rules and impose them on >> > others, like happened some time ago >> > (https://lkml.org/lkml/2012/4/16/473). >> >> You conveniently forgot to show that Linus said that it's OK for a >> subsystem maintainer to request that kind of coding style, it's just >> not OK to impose it tree-wide. > > may I ask some clarification on to what level of subsystem maintainers > this applies. Is this only 1st level subsystem. Or would this extend to > 2nd level subsystems as well? It's a perogative of whatever the desire is of the trees your changes percolate up into on the way to Linus. John may want certain things to look certain ways in the wireless code, and I may have such requirements too. Therefore a wireless change is subjected to both John's and my tastes. I'm really done talking about this, and I'm absolutely not working on documentation or checkpatch.pl changes. Instead I'm going to work on fixing bugs, reviewing patches, and putting together -stable submissions. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-06 18:53 ` David Miller 2012-05-07 8:14 ` Andrei Emeltchenko @ 2012-05-07 17:06 ` Marcel Holtmann 2012-05-07 17:55 ` David Miller 2012-05-07 19:40 ` Lucas De Marchi 2 siblings, 1 reply; 22+ messages in thread From: Marcel Holtmann @ 2012-05-07 17:06 UTC (permalink / raw) To: David Miller Cc: dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless Hi Dave, > > Whose rules are they? > > Find me an example in another major core subsystem, let's use > mm/memory.c as an example as that file gets hit by a lot of people, > that uses the multi-line conditional TAB-only crap you guys seem to > keep using. > > They don't. All the examples you'll find are of the form: > > if (a && > b) > > not: > > if (a && > b) except of course in zap_vma_ptes(), remap_pmd_range(), remap_pud_range() and do_wp_page(). So we also have this one: if ((vma->vm_flags & (VM_WRITE|VM_SHARED)) == (VM_WRITE|VM_SHARED)) goto reuse; And this: } else if (unlikely((vma->vm_flags & (VM_WRITE|VM_SHARED)) == (VM_WRITE|VM_SHARED))) { What kind of style requirement is that one? tmp = vma->vm_ops->page_mkwrite(vma, &vmf); if (unlikely(tmp & (VM_FAULT_ERROR | VM_FAULT_NOPAGE))) { ret = tmp; goto unwritable_page; } Have you actually looked at mm/memory.c and confirmed that it is a good example of multi-line indentation? When it comes to function declaration and function calls, the style in mm/memory.c is mixed. We can start counting, but for both other multi-line cases it seems that tab-only indentation is predominant. Regards Marcel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 17:06 ` Marcel Holtmann @ 2012-05-07 17:55 ` David Miller 2012-05-07 21:22 ` Marcel Holtmann 0 siblings, 1 reply; 22+ messages in thread From: David Miller @ 2012-05-07 17:55 UTC (permalink / raw) To: marcel; +Cc: dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless From: Marcel Holtmann <marcel@holtmann.org> Date: Mon, 07 May 2012 10:06:00 -0700 > When it comes to function declaration and function calls, the style in > mm/memory.c is mixed. We can start counting, but for both other > multi-line cases it seems that tab-only indentation is predominant. Fair enough. But you know what the real issue is? I have told the bluetooth folks this matters to me, repeatedly. And then when I pushback when some unacceptable changes slip through, they put a bullseye on my head and say I'm being unreasonable. What's unreasonable about a maintainer telling you multiple times to do things a certain way and then getting mad when you still don't listen? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 17:55 ` David Miller @ 2012-05-07 21:22 ` Marcel Holtmann 2012-05-07 23:24 ` Joe Perches 0 siblings, 1 reply; 22+ messages in thread From: Marcel Holtmann @ 2012-05-07 21:22 UTC (permalink / raw) To: David Miller Cc: dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless Hi Dave, > > When it comes to function declaration and function calls, the style in > > mm/memory.c is mixed. We can start counting, but for both other > > multi-line cases it seems that tab-only indentation is predominant. > > Fair enough. > > But you know what the real issue is? > > I have told the bluetooth folks this matters to me, repeatedly. > > And then when I pushback when some unacceptable changes slip through, > they put a bullseye on my head and say I'm being unreasonable. > > What's unreasonable about a maintainer telling you multiple times > to do things a certain way and then getting mad when you still > don't listen? I have no problem in getting these fixed and you actually imposing this coding style on all networking subsystem. That is your prerogative. However I do have a problem if this is not documented in CodingStyle or somewhere else as part of the kernel source. It should be documented and people pointing out the lack of this are full in their right to do so. I personally would expect the maintainer enforcing this rule to ensure that it is properly documented. Especially after such a confusion has arisen. That checkpatch.pl needs an extra command line called --subjective or -strict to have this easily tested is really not intuitive. I would love to see the idea realized that checkpatch.pl does the enabling/disabling of coding style warnings automatically based on the path. Anyway, these are my view points on this. Regards Marcel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 21:22 ` Marcel Holtmann @ 2012-05-07 23:24 ` Joe Perches 0 siblings, 0 replies; 22+ messages in thread From: Joe Perches @ 2012-05-07 23:24 UTC (permalink / raw) To: Marcel Holtmann Cc: David Miller, dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless On Mon, 2012-05-07 at 14:22 -0700, Marcel Holtmann wrote: > I would love > to see the idea realized that checkpatch.pl does the enabling/disabling > of coding style warnings automatically based on the path. Hey Marcel. Please try this out: It adds a new "C: " line to MAINTAINERS sections that can add --strict or --ignore=foo to suit specific maintainer taste when using checkpatch.pl I only added --strict to drivers/net/ to test. Play with it and let me know if it works for you. not-signed-off-by: Joe Perches <joe@perches.com> --- MAINTAINERS | 5 +++++ scripts/checkpatch.pl | 34 ++++++++++++++++++++++++++++++++-- scripts/get_maintainer.pl | 17 +++++++++++++++-- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 77fddaf..38c1745 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -84,6 +84,10 @@ Descriptions of section entries: Obsolete: Old code. Something tagged obsolete generally means it has been replaced by a better system and you should be using that. + C: Checkpatch options for this section + Options are used in addition to any command line or + checkpatch.conf arguments. All sections that match + a particular file's F: patterns are used. F: Files and directories with wildcard patterns. A trailing slash includes all files and subdirectory files. F: drivers/net/ all files in and below drivers/net @@ -4727,6 +4731,7 @@ W: http://www.linuxfoundation.org/en/Net T: git git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git T: git git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git S: Odd Fixes +C: --strict F: drivers/net/ F: include/linux/if_* F: include/linux/*device.h diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index cb08290..8b2ac0d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -13,6 +13,7 @@ $P =~ s@.*/@@g; my $V = '0.32'; use Getopt::Long qw(:config no_auto_abbrev); +use Getopt::Long qw(GetOptionsFromString); my $quiet = 0; my $tree = 1; @@ -33,6 +34,7 @@ my %ignore_type = (); my @ignore = (); my $help = 0; my $configuration_file = ".checkpatch.conf"; +my $maintainer_conf = 1; sub help { my ($exitcode) = @_; @@ -51,6 +53,8 @@ Options: -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests --ignore TYPE(,TYPE2...) ignore various comma separated message types + --c, --maintainer_prefs use MAINTAINER C: checkpatch preferences lines + default: true (disable with --noc) --show-types show the message "types" in the output --root=PATH PATH to the kernel tree root --no-summary suppress the per-file summary @@ -105,6 +109,8 @@ GetOptions( 'f|file!' => \$file, 'subjective!' => \$check, 'strict!' => \$check, + 'maintainer_prefs!' => \$maintainer_conf, + 'c!' => \$maintainer_conf, 'ignore=s' => \@ignore, 'show-types!' => \$show_types, 'root=s' => \$root, @@ -1497,6 +1503,7 @@ sub process { $realcnt = 0; $linenr = 0; + my $last_realfile = ""; foreach my $line (@lines) { $linenr++; @@ -1554,11 +1561,15 @@ sub process { # extract the filename as it passes if ($line =~ /^diff --git.*?(\S+)$/) { $realfile = $1; - $realfile =~ s@^([^/]*)/@@; + if (!$file) { + $realfile =~ s@^([^/]*)/@@; + } $in_commit_log = 0; } elsif ($line =~ /^\+\+\+\s+(\S+)/) { $realfile = $1; - $realfile =~ s@^([^/]*)/@@; + if (!$file) { + $realfile =~ s@^([^/]*)/@@; + } $in_commit_log = 0; $p1_prefix = $1; @@ -1577,6 +1588,25 @@ sub process { $here .= "FILE: $realfile:$realline:" if ($realcnt != 0); + if (-f "$realfile" && -f "$root/scripts/get_maintainer.pl" && + $last_realfile ne $realfile) { + $last_realfile = $realfile; + my $m_conf = `perl $root/scripts/get_maintainer.pl -f --checkpatch --noemail --nol --nogit --nogit-fallback $realfile`; + + $m_conf =~ s/\s*\n?$//g; + $m_conf =~ s/^\s*//g; + $m_conf =~ s/\s+/ /g; + + if ($m_conf ne "") { + GetOptionsFromString($m_conf, + 'subjective!' => \$check, + 'strict!' => \$check, + 'ignore=s' => \@ignore, + 'show-types!' => \$show_types, + 'test-only=s' => \$tst_only); + } + } + my $hereline = "$here\n$rawline\n"; my $herecurr = "$here\n$rawline\n"; my $hereprev = "$here\n$prevrawline\n$rawline\n"; diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 0948c6b..98e57ff 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -47,6 +47,7 @@ my $subsystem = 0; my $status = 0; my $keywords = 1; my $sections = 0; +my $checkpatch = 0; my $file_emails = 0; my $from_filename = 0; my $pattern_depth = 0; @@ -206,6 +207,7 @@ if (!GetOptions( 'status!' => \$status, 'scm!' => \$scm, 'web!' => \$web, + 'checkpatch!' => \$checkpatch, 'pattern-depth=i' => \$pattern_depth, 'k|keywords!' => \$keywords, 'sections!' => \$sections, @@ -243,12 +245,13 @@ if ($sections) { $status = 0; $subsystem = 0; $web = 0; + $checkpatch = 0; $keywords = 0; $interactive = 0; } else { - my $selections = $email + $scm + $status + $subsystem + $web; + my $selections = $email + $scm + $status + $subsystem + $web + $checkpatch; if ($selections == 0) { - die "$P: Missing required option: email, scm, status, subsystem or web\n"; + die "$P: Missing required option: email, scm, status, subsystem, web or checkpatch\n"; } } @@ -469,6 +472,7 @@ my %hash_list_to; my @list_to = (); my @scm = (); my @web = (); +my @checkpatch = (); my @subsystem = (); my @status = (); my %deduplicate_name_hash = (); @@ -502,6 +506,11 @@ if ($web) { output(@web); } +if ($checkpatch) { + @checkpatch = uniq(@checkpatch); + output(@checkpatch); +} + exit($exit); sub range_is_maintained { @@ -548,6 +557,7 @@ sub get_maintainers { @list_to = (); @scm = (); @web = (); + @checkpatch = (); @subsystem = (); @status = (); %deduplicate_name_hash = (); @@ -751,6 +761,7 @@ MAINTAINER field selection options: --status => print status if any --subsystem => print subsystem name if any --web => print website(s) if any + --checkpatch => print checkpatch configuration if any Output type options: --separator [, ] => separator for multiple entries on 1 line @@ -1060,6 +1071,8 @@ sub add_categories { push(@web, $pvalue); } elsif ($ptype eq "S") { push(@status, $pvalue); + } elsif ($ptype eq "C") { + push(@checkpatch, $pvalue); } } } ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-06 18:53 ` David Miller 2012-05-07 8:14 ` Andrei Emeltchenko 2012-05-07 17:06 ` Marcel Holtmann @ 2012-05-07 19:40 ` Lucas De Marchi 2 siblings, 0 replies; 22+ messages in thread From: Lucas De Marchi @ 2012-05-07 19:40 UTC (permalink / raw) To: David Miller Cc: dh.herrmann, gustavo, linville, linux-bluetooth, linux-wireless On Sun, May 6, 2012 at 3:53 PM, David Miller <davem@davemloft.net> wrote: > From: David Herrmann <dh.herrmann@googlemail.com> > Date: Sun, 6 May 2012 19:46:46 +0200 > >> Whose rules are they? > > Find me an example in another major core subsystem, let's use The ones he pointed out, coming from Linus aren't enough? > mm/memory.c as an example as that file gets hit by a lot of people, > that uses the multi-line conditional TAB-only crap you guys seem to > keep using. Look again at this file and you'll see there is mixed style. > > They don't. All the examples you'll find are of the form: > > if (a && > b) > > not: > > if (a && > b) > > like I see happening in bluetooth all the time. > > How did you think they figured that out? Did they read someone's > mind? No, they used the default in emacs or another editor that has this as default, i.e. mix tabs and spaces Regards, Lucas De Marchi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-06 16:36 ` David Miller 2012-05-06 17:46 ` David Herrmann @ 2012-05-07 15:24 ` Gustavo Padovan 2012-05-07 15:56 ` David Miller 1 sibling, 1 reply; 22+ messages in thread From: Gustavo Padovan @ 2012-05-07 15:24 UTC (permalink / raw) To: David Miller; +Cc: linville, linux-bluetooth, linux-wireless * David Miller <davem@davemloft.net> [2012-05-06 12:36:56 -0400]: > From: Gustavo Padovan <gustavo@padovan.org> > Date: Fri, 4 May 2012 15:59:48 -0300 > > > Fix offending styles all over the tree. This is in conformance David > > Miller's style rules. > > They aren't just my style rules. > > And, if your plan is to patch things up afterwards so you don't > have to change existing commits in your tree, think again. > > I absolutely will not pull a tree from John that has commits in > it that add the bad indentation. > > You have to respin your tree with fixed commits. John, can you please unpull the bluetooth tree from wireless-next? I'll be fixing he affected commits and the send you a new pull request. Thanks. Gustavo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Bluetooth: Fix coding style 2012-05-07 15:24 ` Gustavo Padovan @ 2012-05-07 15:56 ` David Miller 0 siblings, 0 replies; 22+ messages in thread From: David Miller @ 2012-05-07 15:56 UTC (permalink / raw) To: gustavo; +Cc: linville, linux-bluetooth, linux-wireless From: Gustavo Padovan <gustavo@padovan.org> Date: Mon, 7 May 2012 12:24:05 -0300 > * David Miller <davem@davemloft.net> [2012-05-06 12:36:56 -0400]: > >> From: Gustavo Padovan <gustavo@padovan.org> >> Date: Fri, 4 May 2012 15:59:48 -0300 >> >> > Fix offending styles all over the tree. This is in conformance David >> > Miller's style rules. >> >> They aren't just my style rules. >> >> And, if your plan is to patch things up afterwards so you don't >> have to change existing commits in your tree, think again. >> >> I absolutely will not pull a tree from John that has commits in >> it that add the bad indentation. >> >> You have to respin your tree with fixed commits. > > John, can you please unpull the bluetooth tree from wireless-next? I'll be > fixing he affected commits and the send you a new pull request. Thanks. And John also make sure these guys don't add unreasonable commit log messages saying things like that this multi-line condition style thing is my specific coding style. It's not. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2012-05-07 23:24 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-04 18:59 [PATCH] Bluetooth: Fix coding style Gustavo Padovan 2012-05-06 16:02 ` Marcel Holtmann 2012-05-06 16:36 ` David Miller 2012-05-06 17:46 ` David Herrmann 2012-05-06 18:53 ` David Miller 2012-05-07 8:14 ` Andrei Emeltchenko 2012-05-07 8:21 ` Arend van Spriel 2012-05-07 8:29 ` Andrei Emeltchenko 2012-05-07 10:06 ` Arend van Spriel 2012-05-07 15:52 ` David Miller 2012-05-07 19:49 ` Lucas De Marchi 2012-05-07 20:06 ` David Miller 2012-05-07 20:14 ` Arend van Spriel 2012-05-07 21:33 ` Marcel Holtmann 2012-05-07 21:41 ` David Miller 2012-05-07 17:06 ` Marcel Holtmann 2012-05-07 17:55 ` David Miller 2012-05-07 21:22 ` Marcel Holtmann 2012-05-07 23:24 ` Joe Perches 2012-05-07 19:40 ` Lucas De Marchi 2012-05-07 15:24 ` Gustavo Padovan 2012-05-07 15:56 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).