From mboxrd@z Thu Jan 1 00:00:00 1970 From: wagi@monom.org (Daniel Wagner) Date: Mon, 29 Jul 2013 16:58:29 +0200 Subject: [Cocci] Formatting issue In-Reply-To: References: <51E6B08E.9030600@monom.org> <51E6B532.8010909@monom.org> <20130717155132.GA20499@domone.kolej.mff.cuni.cz> <51F62B0C.2040906@monom.org> <51F630D7.1070701@monom.org> Message-ID: <51F68315.3020806@monom.org> To: cocci@systeme.lip6.fr List-Id: cocci@systeme.lip6.fr 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