Opened 5 years ago

Closed 5 years ago

#13172 closed defect (implemented)

Stop using operators as macro arguments ?

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The "coccinelle" semantic patch tool really gets confused when we use operators as macro arguments. It can't modify any function that does that. We use operators as macro arguments in two ways:

  1. We pass them to timercmp.
  1. We pass them as arguments to our tt_*_op() macros.

It's pretty trivial to make this change: We just need to add "#define LT <" and so forth, and then we can use a perl script ro replace "<," and "<)" with "LT". Then so on for GT, EQ, NEQ, GEQ, LEQ.

Is this worthwhile?

Child Tickets

Change History (5)

comment:1 Changed 5 years ago by nickm

Status: newneeds_review

branch 'ticket13172' in my public repository has a changes file and the necessary new macros. After merging that, we should run this perl script over the code:

#!/usr/bin/perl -w -p -i

next if m#^ */\*# or m#^ *\* #;

s/<([,)])/OP_LT$1/;
s/(?<=[\s,])>([,)])/OP_GT$1/;
#s/>([,)])/OP_GT$1/;
s/==([,)])/OP_EQ$1/;
s/>=([,)])/OP_GE$1/;
s/<=([,)])/OP_LE$1/;
s/!=([,)])/OP_NE$1/;

The rigamarole about skipping obvious comments and continuations, and the complications on the OP_GT macro, are meant to keep us from changing HTML elements in doxygen comments.

I believe that asn and dgoulet like this idea. Does anybody object?

Last edited 5 years ago by nickm (previous) (diff)

comment:2 Changed 5 years ago by dgoulet

The defines in ticket13172 are ok.

My perl ninja is real bad so I'll let you be the judge that everything is indeed modified with that.

Quick grep, I found this that has a space between the op and ",". Not sure the regex above covers it. Only one I found FYI, I have tested ' ,' and ' )'.

./src/ext/tinytest_demo.c +80 	tt_int_op(strcmp("abc", "abcd"), < , 0);

comment:3 Changed 5 years ago by nickm

Possibly better perl:

#!/usr/bin/perl -w -p -i

next if m#^ */\*# or m#^ \*#;

s/< *([,)])/OP_LT$1/;
s/(?<=[\s,])< *([,)])/OP_GT$1/;
#s/> *([,)])/OP_GT$1/;
s/== *([,)])/OP_EQ$1/;
s/>= *([,)])/OP_GE$1/;
s/<= *([,)])/OP_LE$1/;
s/!= *([,)])/OP_NE$1/;

comment:4 Changed 5 years ago by nickm

I've heard no objections here, so I'll start the madness.

comment:5 Changed 5 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged.

Note: See TracTickets for help on using tickets.