-
Notifications
You must be signed in to change notification settings - Fork 284
Limit class loading #604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Limit class loading #604
Conversation
64b5643 to
346d57e
Compare
|
What's the advantage of that in comparison to lazy class loading (#407)? |
|
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. |
|
I see. These two are orthogonal. |
|
cegis needs json.a to compile: https://travis-ci.org/diffblue/cbmc/jobs/208614294#L2370 |
fb4917f to
8e96c2a
Compare
|
added json.a to cegis Makefile |
tautschnig
left a comment
There was a problem hiding this 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.
src/java_bytecode/jar_file.cpp
Outdated
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/java_bytecode/jar_file.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is iostream really required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/java_bytecode/jar_file.cpp
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/java_bytecode/jar_file.cpp
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/java_bytecode/jar_file.cpp
Outdated
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
src/java_bytecode/jar_file.h
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/java_bytecode/jar_file.h
Outdated
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A blank line, please.
e.g.
```
{
"jar":
[
"A.jar",
"B.jar"
],
"classFiles":
[
"jarfile3$A.class",
"jarfile3.class"
]
}
```
8e96c2a to
1975394
Compare
eae1b8b to
53fad70
Compare
…p_on_exception [SEC-677] Turning DI off by default and making errors fail pipeline
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-filesparameter.requires #548