@headius suggested to report the rest of our findings as one issue. There is not much through and they are minor i suppose. Others are #8807 #8809 #8811 #8812 #8814 #8815 #8816 #8818
These findings were confusing, so i can't provide PRs, it's better to discuss them, maybe i didn't get the code logic right.
Possible dead code at EncodingUtils:encSetDefaultEncoding()
|
if (def_p != null) { |
|
overridden = true; |
|
} |
|
|
|
if (encoding.isNil()) { |
|
def_p[0] = null; |
|
// don't set back into encoding table since it defers to us |
|
} else { |
|
def_p[0] = rbToEncoding(context, encoding); |
This method is called from rbEncSetDefaultInternal() and rbEncSetDefaultExternal(), they create enc_p/def_p array, so it is never null. And if some how appear to be null, then we will get NPE.
I don't know the right solution, maybe moving curve bracket:
public static boolean encSetDefaultEncoding(ThreadContext context, Encoding[] def_p, IRubyObject encoding, String name) {
boolean overridden = false;
if (def_p != null) {
overridden = true;
if (encoding.isNil()) {
def_p[0] = null;
// don't set back into encoding table since it defers to us
} else {
def_p[0] = rbToEncoding(context, encoding);
// don't set back into encoding table since it defers to us
}
}
if (name.equals("external")) {
context.runtime.setDefaultFilesystemEncoding(rbToEncoding(context, encoding));
}
return overridden;
}
Possible NPE in BasicCompilerPassListener:endExecute()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/passes/BasicCompilerPassListener.java#L34-L41
fic.toStringInstrs() uses cfg.toStringGraph() and cfg.toStringInstrs(), so if fic.getCFG() is null, then we will get NPE here. Don't know how to fix, maybe simply remove this part.
Strange code in RubyIO:initialize_copy()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyIO.java#L2294-L2295
-1 is illegal argument for seek if i correctly understand the doc. Nothing falls and -1 will be returned.
But maybe there is a mistype and code should look like:
if (pos != -1)
fptr.seek(context, pos, PosixShim.SEEK_SET);
Useless addition in Profiler:analyzeProfile()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/interpreter/Profiler.java#L96-L106
After addition of x c is rewritten by scopeCounts.get(s) in cycle, and after that no use of c i see. Maybe c += x; sould be removed. Or counters are counting something wrong or not updated.
Double buildProcess() call in ShellLauncher:runExternalAndWait()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/ShellLauncher.java#L487-L503
I suppose bad refactoring, and code should look like:
if (cfg.shouldRunInShell()) {
log(runtime, "Launching with shell");
// execute command with sh -c ... this does shell expansion of wildcards
cfg.verifyExecutableForShell();
} else {
log(runtime, "Launching directly (no shell)");
cfg.verifyExecutableForDirect();
}
final String[] execArgs = cfg.getExecArgs();
if (changeDirInsideJar(runtime, execArgs)) {
pwd = new File(System.getProperty("user.dir"));
}
process = buildProcess(runtime, execArgs, getCurrentEnv(runtime, mergeEnv), pwd);
Environment Information
We are analyzing versions 9.4.x (8-12), but this problem is still in master, links provided.
@headius suggested to report the rest of our findings as one issue. There is not much through and they are minor i suppose. Others are #8807 #8809 #8811 #8812 #8814 #8815 #8816 #8818
These findings were confusing, so i can't provide PRs, it's better to discuss them, maybe i didn't get the code logic right.
Possible dead code at EncodingUtils:encSetDefaultEncoding()
jruby/core/src/main/java/org/jruby/util/io/EncodingUtils.java
Lines 1868 to 1876 in ad7c54f
This method is called from rbEncSetDefaultInternal() and rbEncSetDefaultExternal(), they create enc_p/def_p array, so it is never null. And if some how appear to be null, then we will get NPE.
I don't know the right solution, maybe moving curve bracket:
Possible NPE in BasicCompilerPassListener:endExecute()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/passes/BasicCompilerPassListener.java#L34-L41
fic.toStringInstrs() uses cfg.toStringGraph() and cfg.toStringInstrs(), so if fic.getCFG() is null, then we will get NPE here. Don't know how to fix, maybe simply remove this part.
Strange code in RubyIO:initialize_copy()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyIO.java#L2294-L2295
-1 is illegal argument for seek if i correctly understand the doc. Nothing falls and -1 will be returned.
But maybe there is a mistype and code should look like:
Useless addition in Profiler:analyzeProfile()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/interpreter/Profiler.java#L96-L106
After addition of
xcis rewritten byscopeCounts.get(s)in cycle, and after that no use ofci see. Maybec += x;sould be removed. Or counters are counting something wrong or not updated.Double buildProcess() call in ShellLauncher:runExternalAndWait()
https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/ShellLauncher.java#L487-L503
I suppose bad refactoring, and code should look like:
Environment Information
We are analyzing versions 9.4.x (8-12), but this problem is still in master, links provided.