Add sufficient coccinelle tooling to run coccinelle without stress

I think we need two pieces of coccinelle tooling to be able to use it effectively:

1. A script that tells us which files have parsing problems.

2. A script to invoke spatch with the right arguments.

Based on 1 and 2, we can improve our tor-coccinelle.h file to handle more of our codebase, and we can apply coccinelle scripts without trying to remember the name of the "-macro-file-builtins" flag.

changed milestone to %Tor: 0.4.3.x-final

added 042-should actualpoints::1.5 component::core tor/tor milestone::Tor: 0.4.3.x-final owner::nickm priority::medium resolution::fixed reviewer::teor severity::normal sponsor::31-can status::closed type::task labels

See branch ticket31705 with PR at https://github.com/torproject/tor/pull/1321

CI has passed

Trac:
Status: assigned to needs_review

Trac:
Reviewer: N/A to teor

Trac:
Keywords: N/A deleted, 042-can added

Design review:

The design and overall structure looks good. I am not that familiar with coccinelle, so I will need to run these scripts myself as part of my review.

Should we be running these scripts as part of our code quality checks? (We could install coccinelle on Travis, so it runs them.) Is that a separate ticket?

Trac:
Status: needs_review to needs_revision

Is that a separate ticket?

I think/hope that's a separate ticket.

The make check is now #31919 (moved)

(Does this still belong in needs_revision?)

Trac:
Status: needs_revision to needs_review

I left some comments on the ticket:

• document tor-coccinelle.h
• document test_operator_cleanup
• make try_parse.sh exit with a non-zero status if it finds any files
• make (a mode for) try_parse.sh that shows the parsing errors, so they can be fixed
• it looks like you missed some files:

$ find src/{app,config,core,feature,lib,test,tools,win32} -name *.c -o -name *.h | xargs ~/dev/tor/scripts/coccinelle/try_parse.sh
# 143 files

Reminder: please fill in actual points on this ticket.

Trac:
Status: needs_review to needs_revision

Trac:
Keywords: 042-can deleted, 042-should added

I have fixed all of the issues above, and gotten the number of files with parsing errors down from 143 to 12. The remaining unhandled files are:

• Those with funny case statement macros:
  • src/core/mainloop/connection.c
  • src/core/or/reasons.c
  • src/lib/tls/tortls.h
• Those that use SMARTLIST_FOREACH_JOIN():
  • src/feature/nodelist/networkstatus.c
• Those that consist entirely or almost entirely of macro definitions that Coccinelle does not understand:
  • src/lib/pubsub/pubsub_macros.h
  • src/lib/testsupport/testsupport.h
  • src/lib/smartlist_core/smartlist_foreach.h
  • src/lib/container/order.c
  • src/lib/container/handles.h
  • src/lib/container/map.c
  • src/lib/container/map.h
  • src/lib/cc/compat_compiler.h

Also, in some cases, I had to disable parsing of certain things with an #ifdef COCCI macro. In the future, if we can figure out how to parse those things, we should use this macro less often.

After these changes, I ran into some merge conflicts with master and its improved annotate_ifdef_directives. I have made a new branch ticket31705_merged to handle those conflicts, which has a PR at https://github.com/torproject/tor/pull/1403 . If CI passes, I'll call it needs_review.

Trac:
Actualpoints: N/A to 1.5

Trac:
Status: needs_revision to needs_review

Seems fine, but there are some typos and minor commit message/content issues.

How do you want to make progress on the remaining files? Do you want to set up CI, so we preserve cocci compatibility over time?

Trac:
Status: needs_review to needs_revision

Replying to teor:

Seems fine, but there are some typos and minor commit message/content issues.

I've added a fixup commits for the typos. To solve the commit message/content issues, I had to squash those and make a new rebased branch, as ticket31705_v2. This still had merge conflicts with master, so it is merged in a ticket31705_v2_merged branch. The PR for that is https://github.com/torproject/tor/pull/1447 .

How do you want to make progress on the remaining files? Do you want to set up CI, so we preserve cocci compatibility over time?

I don't have a strong opinion here right now: I think it would make more sense to get some experience using coccinelle more in our regular development workflow, so we can decide how to prioritize these items.

Trac:
Status: needs_revision to needs_review

I would like to check the fixed files using cocci in CI. If we don't, they will degrade over time. The CI shouldn't take long, I'll see if I can get it done today,

We can exclude the files that are hard to fix, and open a ticket to deal with them later.

Trac:
Milestone: Tor: 0.4.2.x-final to Tor: 0.4.3.x-final

This is merge_ready.

But there are actually 22 files that don't pass, see: https://github.com/torproject/tor/pull/1454/files#diff-8eaf87fc1c11a68a2909b7da648da0a9

I'm going to wait until #31919 (moved) passes before merging, just in case there are any more surprises.

Trac:
Status: needs_review to merge_ready

CI runs fine in #31919 (moved), just tweaking the performance now.

Merged to master.

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed

added 12h of time spent

mentioned in issue #31919 (moved)

moved to tpo/core/tor#31705 (closed)

mentioned in issue tpo/core/tor#31919 (closed)