From: wagi@monom.org (Daniel Wagner)
To: cocci@systeme.lip6.fr
Subject: [Cocci] Formatting issue
Date: Mon, 29 Jul 2013 16:58:29 +0200 [thread overview]
Message-ID: <51F68315.3020806@monom.org> (raw)
In-Reply-To: <alpine.DEB.2.02.1307291110020.2271@hadrien>
On 07/29/2013 11:14 AM, Julia Lawall wrote:
> On Mon, 29 Jul 2013, Daniel Wagner wrote:
>
>> On 07/29/2013 10:49 AM, Julia Lawall wrote:
>>>>> I now try to convince others to use first option as this allows all
>>>>> submitted patches have perfect formatting because formatter took care of
>>>>> it. This can save few round trips when reviewer wants space after comma
>>>>> etc.
>>>>
>>>> The main problem I see is as soon you start modifing the output from
>>>> spatch by
>>>> hand you risk to introduce an error and as soon one does several rounds of
>>>> review it one will introduce an error.
>>>>
>>>> For the changes in ConnMan I did applied the semantic patch and then
>>>> went over the code base via checkpatch.pl and fixed up the errors.
>>>>
>>>> I don't know if a simple rule like 80 chars max couldn't be added for
>>>> the --linux-spacing switch. That was the offender in my case. The
>>>> rest of the patch was okay.
>>>
>>> It's a simple rule in theory, but not so simple in practice. Pretty
>>> printing is a hard problem, because doing a good job requires knowing both
>>> what you have seen before and what is coming up. Also, when Coccinelle is
>>> generating code, it doesn't see the AST any more, only tokens, so it has
>>> limited information.
>>
>> I understand that in reality this is a hard problem. Something what I would
>> helpful here would be a list of lines (emacs/vim parseable) where the the
>> result line is too long, presumed that detecting of long lines is easy.
>
> Perhaps there already exists a tool or emacs mode to do this?
Yes, as I said I was falling back to use checkpatch.pl to find the
offending lines. My point is that coccinelle already knows where it did
write too long lines, why not just report it?
>>> In any case, the huge indent was just a bug, so hopefully the result will
>>> be somewhat better in the future.
>>
>> Yes, that one is gone. Thanks again.
>>
>> Okay, maybe I am overdoing it with my wishlist. I am trying to prepare another
>> semantic patch and I see that coccinelle's parser is not too happy with
>> certain code. Most of the time I just need to add the right macro to my
>> standard.h to make coccinelle happy. My wish would be that coccinelle wouldn't
>> just ignore that file instead just stop and yell at me: "Parser error: ...".
>> Did I overlook such an option?
>
> It doesn't do this by default, because almost all files contain things it
> can't parse, and most of the time the problem is not related to the code
> you want to process.
>
> You can use the --verbose-parsing option to get all
> of the information about parsing. But there is a lot of it, and you
> should know that Coccinelle makes a number of attempts to parse each
> function, and so you can get parse errors on the earlier attempts even
> when it succeeds on later attempts.
>
> Usually, it is sufficient to run spatch -parse_c {your code} a few times,
> and see what are the top 10 failure tokens that it reports at the end.
Right, '-parse_c' was what I was looking for. Now I see where the parse
finally chokes.
Hmm, a simple define is too much:
N6ADDR_ALL_DHCP_RELAY_AGENTS_AND_SERVERS_MC_INIT: present in 1 parsing
errors
example:
#define IN6ADDR_ALL_DHCP_RELAY_AGENTS_AND_SERVERS_MC_INIT \
{ { { 0xff,0x02,0,0,0,0,0,0,0,0,0,0,0,0x1,0,0x2 } } } /*
ff02::1:2 */
static const struct in6_addr
in6addr_all_dhcp_relay_agents_and_servers_mc =
https://git.kernel.org/cgit/network/connman/connman.git/tree/gdhcp/common.c#n465
cheers,
daniel
next prev parent reply other threads:[~2013-07-29 14:58 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-17 14:56 [Cocci] Formatting issues Daniel Wagner
2013-07-17 15:07 ` Julia Lawall
2013-07-17 15:16 ` Daniel Wagner
2013-07-17 15:31 ` Julia Lawall
2013-07-17 15:41 ` Daniel Wagner
2013-07-17 15:51 ` Ondřej Bílka
2013-07-29 8:42 ` Daniel Wagner
2013-07-29 8:49 ` Julia Lawall
2013-07-29 9:07 ` Daniel Wagner
2013-07-29 9:14 ` Julia Lawall
2013-07-29 14:58 ` Daniel Wagner [this message]
2013-07-29 9:28 ` Ondřej Bílka
2013-07-29 14:59 ` Daniel Wagner
2013-07-29 15:59 ` Ondřej Bílka
2013-07-30 21:06 ` Daniel Wagner
2013-07-28 10:48 ` Julia Lawall
2013-07-29 8:33 ` Daniel Wagner
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=51F68315.3020806@monom.org \
--to=wagi@monom.org \
--cc=cocci@systeme.lip6.fr \
/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