Coccinelle Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  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