Skip to content

support withincode pointcut in the event definition#257

Merged
yzhang90 merged 1 commit intomasterfrom
withincode
Apr 14, 2020
Merged

support withincode pointcut in the event definition#257
yzhang90 merged 1 commit intomasterfrom
withincode

Conversation

@yzhang90
Copy link
Copy Markdown
Contributor

No description provided.

@yzhang90 yzhang90 requested a review from msaxena2 April 10, 2020 15:45

public R visit(WithinPointCut p, A arg);

public R visit(WithincodePointCut p, A arg);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: One more horizontal space

Comment on lines +92 to +95
public PointCut visit(WithincodePointCut p, Object arg) {
return p;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more horizontal space

@owolabileg
Copy link
Copy Markdown
Contributor

owolabileg commented Apr 10, 2020

  1. What's the use case (e.g., an example spec) that requires a withincode pointcut?
  2. There are several other cases where more horizontal space should be added to new code, for consistency. I pointed out a couple.
  3. What's the maximum number of imports from the same package before we import * from that package? I see several cases where the new changes imports * for relatively few imports. Perhaps keep the separate imports?
  4. Please check that these are all the places that need to be changed, e.g., is withincode in a raw spec parsed properly?

@yzhang90
Copy link
Copy Markdown
Contributor Author

  1. What's the use case (e.g., an example spec) that requires a withincode pointcut?
    The withincode is used to pinpoint the exact instrumentation location. For example, I only want to instrument before db.put method when it is called inside dbStorage.put method.
    event putdbLock before ():
        call(* HaloDB.put(byte[], byte[]))
        && withincode(* HaloDBStorageEngine.put(byte[], byte[], int)) {
            System.out.println("Before acquire the putLock");
            dbPutLock.lock();
            System.out.println("After acquire the putLock");
    }

in this event we want to synchronize the db.put method and monitor state update code in the HaloDBStorageEngine.put(..) function.

  1. Please check that these are all the places that need to be changed, e.g., is withincode in a raw spec parsed properly?
    I tried this change on a raw spec.

@yzhang90
Copy link
Copy Markdown
Contributor Author

  1. What's the maximum number of imports from the same package before we import * from that package? I see several cases where the new changes imports * for relatively few imports. Perhaps keep the separate imports?
    I change them to imports * because in the visitor class, we basically import all the class from the ast package. Do we need to list them one by one in the header?

@msaxena2
Copy link
Copy Markdown
Contributor

@yzhang90 Looks good, apart from the formatting issue which has already been mentioned -

  1. Seems like there is a file for unit testing the parser, although the number of tests seems to be extremely limited. Perhaps we can start improving the testing situation by adding a test for withincode?
  2. Also extend the integration test using a simple spec that exercises withincode? This perhaps lies beyond the scope of this PR, since the integration testing may require a little more setup.

@yzhang90 yzhang90 merged commit 2126110 into master Apr 14, 2020
@yzhang90 yzhang90 deleted the withincode branch April 14, 2020 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants