Skip to content

Conversation

@mgudemann
Copy link
Contributor

@mgudemann mgudemann commented Mar 4, 2017

This PR adds the capability to load only the subset of class files from a jar which match a regex specified via the java-cp-include-files parameter.
requires #548

@mgudemann mgudemann force-pushed the limit_class_loading branch from 64b5643 to 346d57e Compare March 4, 2017 13:18
@peterschrammel
Copy link
Member

What's the advantage of that in comparison to lazy class loading (#407)?

@mgudemann
Copy link
Contributor Author

It's in addition to lazy loading. If I understand correctly, lazy loading will only load the parts that are actually used and not everything that is present. This allows you to limit beforehand what is seen from a jar file, @marek-trtik had asked for such a feature.

@peterschrammel
Copy link
Member

I see. These two are orthogonal.

@mgudemann mgudemann mentioned this pull request Mar 6, 2017
@peterschrammel
Copy link
Member

cegis needs json.a to compile: https://travis-ci.org/diffblue/cbmc/jobs/208614294#L2370

@mgudemann mgudemann force-pushed the limit_class_loading branch from fb4917f to 8e96c2a Compare March 8, 2017 08:58
@mgudemann
Copy link
Contributor Author

added json.a to cegis Makefile

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

Various requests for cleanup and logic simplifications.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit picking: group STL headers, only then start with local headers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is iostream really required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not an internal error (it's invalid user input), I think - should be throw instead of assert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

As above: invalid user input, not an internal error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why use count() instead of find?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it was shorter, also it is an unordered map so it shouldn't be less efficient as count can only be 0 or 1 and if an element is found, the implemention likely stops traversing the unordered map. In any case, I change this to find() to be consistent with the code base.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a user input, I believe?! Should be thrown, not asserted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does jar_filet benefit from being a messaget (see comments above)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a status output to see about hidden files, also the JSON parser needs a valid one

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a blank line. Codewithoutblanksisreallyhardtoread.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See above for why I believe these aren't internal errors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A blank line, please.

Matthias Güdemann added 4 commits March 9, 2017 18:30
@mgudemann mgudemann force-pushed the limit_class_loading branch from 8e96c2a to 1975394 Compare March 10, 2017 10:19
@mgudemann mgudemann force-pushed the limit_class_loading branch from eae1b8b to 53fad70 Compare March 10, 2017 13:31
@kroening kroening merged commit 36c2e2d into diffblue:master Mar 13, 2017
NathanJPhillips pushed a commit to NathanJPhillips/cbmc that referenced this pull request Sep 6, 2018
…p_on_exception

[SEC-677] Turning DI off by default and making errors fail pipeline
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants