Opened 7 years ago

Closed 6 years ago

#981 closed task (complete)

The NOT ACL operator

Reported by: vorner Owned by: vorner
Priority: medium Milestone: Sprint-20110712
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket: ACL
Estimated Difficulty: 2.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

When we have the loader (#978) to load the subexpression, we want to implement the NOT logic operator. This contains both the ACL class itself and a factory function for it.

Subtickets

Change History (9)

comment:1 Changed 6 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 2

comment:2 Changed 6 years ago by stephen

  • Milestone changed from Next-Sprint-Proposed to Sprint-20110712

comment:3 Changed 6 years ago by vorner

  • Owner set to vorner
  • Status changed from new to accepted

comment:4 Changed 6 years ago by vorner

  • Owner changed from vorner to UnAssigned
  • Status changed from accepted to reviewing

Hello

This should be straightforward and ready for review.

I registered the ANY and ALL operators when I was registering the NOT operator, simply because it was forgotten before and creating a new ticket would be more work than copy-pasting the NOT registration two more times.

comment:5 Changed 6 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:6 Changed 6 years ago by stephen

  • Owner changed from stephen to vorner

Reviewed commit ea2a4f906dc3b5bae939a0348a5f82fa690bbec5. Just one comment:

src/lib/acl/logic_check.h
It does occur to me that there is now an inconsistency in class names. Logic operators include the operations AND, OR and NOT. Only AND and OR (i.e. All and Any) are handled by the class LogicOperator. NOT is handled by the class NotCheck. For consistency, NotCheck should be renamed NotOperator (or LogicOperator renamed to LogicCheck).

(Ideally, LogicOperator should also be renamed to indicate that it is restricted to just AND and OR, but at the moment I can't think of a better name.)

comment:7 Changed 6 years ago by vorner

  • Owner changed from vorner to stephen

Yes, you're right, I renamed it.

Technically, the LogicOperator? could do even more operators (the Not one is special, because it is unary), but I guess none other makes real sense. And MultiAryOperator? sounded stupid to me, so it ended up as logic operator.

Thanks

comment:8 Changed 6 years ago by stephen

  • Owner changed from stephen to vorner

OK, you can merge although I got an error running the server_common tests. This was fixed by adding liblog to the list of libraries linked against in the Makefile.am; I have pushed this change.

OK to merge.

comment:9 Changed 6 years ago by vorner

  • Resolution set to complete
  • Status changed from reviewing to closed

Thanks, merged.

Note: See TracTickets for help on using tickets.