Skip to content

Commit e747b5e

Browse files
GregDomjanGregDomjan
authored
Merge pull request #394 from susanw1/master
Handle NPEs better - config errors logged as Warn; mitigate usage errors
2 parents 2ed4eff + 4f8e602 commit e747b5e

File tree

4 files changed

+63
-52
lines changed

4 files changed

+63
-52
lines changed

‎src/main/java/com/github/maven_nar/Compiler.java

+42-32
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,32 @@ public abstract class Compiler {
250250
protected Compiler() {
251251
}
252252

253+
/**
254+
* Parses a "NAME=VALUE" string into a name and (optional) value. Logs to warn if argument is null, implying a blank
255+
* config item (eg <define></define>, or <define>${x}</define> when x is undefined).
256+
*
257+
* @param argument the string to parse (may be null)
258+
* @param configName the name of the argument type, for logging purposes
259+
* @return the parsed DefineArgument, or null if argument is null
260+
*/
261+
private DefineArgument parseDefineArgument(final String argument, String configName) {
262+
if (argument == null) {
263+
this.mojo.getLog().warn("NAR configuration: empty " + configName + " - ignoring");
264+
return null;
265+
}
266+
final String[] pair = argument.split("=", 2);
267+
268+
if (pair[0].equals("")) {
269+
this.mojo.getLog().warn("NAR configuration: definition has no name in " + configName + " - ignoring");
270+
return null;
271+
}
272+
273+
final DefineArgument define = new DefineArgument();
274+
define.setName(pair[0]);
275+
define.setValue(pair.length > 1 ? cleanDefineValue(pair[1]) : null);
276+
return define;
277+
}
278+
253279
/**
254280
* Filter elements such as cr\lf that are problematic when used inside a `define`
255281
*
@@ -328,13 +354,10 @@ public final CompilerDef getCompiler(final String type, final String output)
328354
}
329355

330356
if (this.optionSet != null) {
331-
332357
final String[] opts = this.optionSet.split("\\s");
333358

334359
for (final String opt : opts) {
335-
336360
final CompilerArgument arg = new CompilerArgument();
337-
338361
arg.setValue(opt);
339362
compilerDef.addConfiguredCompilerArg(arg);
340363
}
@@ -358,31 +381,24 @@ public final CompilerDef getCompiler(final String type, final String output)
358381
if (this.defines != null) {
359382
final DefineSet ds = new DefineSet();
360383
for (final String string : this.defines) {
361-
final DefineArgument define = new DefineArgument();
362-
final String[] pair = string.split("=", 2);
363-
define.setName(pair[0]);
364-
define.setValue(pair.length > 1 ? cleanDefineValue(pair[1]) : null);
365-
ds.addDefine(define);
384+
final DefineArgument define = parseDefineArgument(string, "define");
385+
if (define != null) {
386+
ds.addDefine(define);
387+
}
366388
}
367389
compilerDef.addConfiguredDefineset(ds);
368390
}
369391

370392
if (this.defineSet != null) {
371-
372393
final String[] defList = this.defineSet.split(",");
373394
final DefineSet defSet = new DefineSet();
374395

375396
for (final String element : defList) {
376-
377-
final String[] pair = element.trim().split("=", 2);
378-
final DefineArgument def = new DefineArgument();
379-
380-
def.setName(pair[0]);
381-
def.setValue(pair.length > 1 ? cleanDefineValue(pair[1]) : null);
382-
383-
defSet.addDefine(def);
397+
final DefineArgument define = parseDefineArgument(element, "defineSet");
398+
if (define != null) {
399+
defSet.addDefine(define);
400+
}
384401
}
385-
386402
compilerDef.addConfiguredDefineset(defSet);
387403
}
388404

@@ -400,11 +416,10 @@ public final CompilerDef getCompiler(final String type, final String output)
400416
if (this.undefines != null) {
401417
final DefineSet us = new DefineSet();
402418
for (final String string : this.undefines) {
403-
final DefineArgument undefine = new DefineArgument();
404-
final String[] pair = string.split("=", 2);
405-
undefine.setName(pair[0]);
406-
undefine.setValue(pair.length > 1 ? pair[1] : null);
407-
us.addUndefine(undefine);
419+
final DefineArgument undef = parseDefineArgument(string, "undefine");
420+
if (undef != null) {
421+
us.addUndefine(undef);
422+
}
408423
}
409424
compilerDef.addConfiguredDefineset(us);
410425
}
@@ -415,16 +430,11 @@ public final CompilerDef getCompiler(final String type, final String output)
415430
final DefineSet undefSet = new DefineSet();
416431

417432
for (final String element : undefList) {
418-
419-
final String[] pair = element.trim().split("=", 2);
420-
final DefineArgument undef = new DefineArgument();
421-
422-
undef.setName(pair[0]);
423-
undef.setValue(pair.length > 1 ? pair[1] : null);
424-
425-
undefSet.addUndefine(undef);
433+
final DefineArgument undef = parseDefineArgument(element, "undefineSet");
434+
if (undef != null) {
435+
undefSet.addUndefine(undef);
436+
}
426437
}
427-
428438
compilerDef.addConfiguredDefineset(undefSet);
429439
}
430440

‎src/main/java/com/github/maven_nar/IncludePath.java

+5
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,9 @@ public void setPath(final String path) {
8585
this.path = path;
8686
this.file = new File(path);
8787
}
88+
89+
@Override
90+
public String toString() {
91+
return getPath() + (includes == null ? "" : "[" + getIncludes() + "]");
92+
}
8893
}

‎src/main/java/com/github/maven_nar/Linker.java

+12-15
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
*/
2020
package com.github.maven_nar;
2121

22+
import static com.google.common.base.Strings.isNullOrEmpty;
23+
import static com.google.common.base.Strings.nullToEmpty;
24+
2225
import java.io.File;
2326
import java.io.IOException;
2427
import java.util.HashSet;
2528
import java.util.LinkedList;
2629
import java.util.List;
27-
import java.util.Properties;
2830
import java.util.Set;
2931
import java.util.Arrays;
3032
import java.util.Collections;
@@ -283,27 +285,27 @@ public final LinkerDef getLinker(final AbstractCompileMojo mojo, final CCTask ta
283285
linker.setSkipDepLink(this.skipDepLink);
284286

285287
// incremental, map
286-
String linkerPrefix;
287-
if (this.prefix == null || this.prefix.equals("")) {
288+
final String linkerPrefix;
289+
// don't add prefix to ar-like commands FIXME should be done in cpptasks
290+
if (type.equals(Library.STATIC) && !getName(null, null).equals("msvc")) {
291+
linkerPrefix = null;
292+
}
293+
else if (isNullOrEmpty(this.prefix)) {
288294
String key = mojo.getAOL().getKey() + ".linker.prefix";
289295
linkerPrefix = NarProperties.getInstance(mojo.getMavenProject()).getProperty(key);
290296
}
291297
else {
292298
linkerPrefix = this.prefix;
293299
}
294300

295-
// don't add prefix to ar-like commands FIXME should be done in cpptasks
296-
if (type.equals(Library.STATIC) && !getName(null, null).equals("msvc")) {
297-
linkerPrefix = null;
298-
}
299301
linker.setLinkerPrefix(linkerPrefix);
300302
linker.setIncremental(this.incremental);
301303
linker.setMap(this.map);
302304

303305
// Add definitions (Window only)
304306
if (os.equals(OS.WINDOWS) && getName(null, null).equals("msvc")
305307
&& (type.equals(Library.SHARED) || type.equals(Library.JNI))) {
306-
final Set defs = new HashSet();
308+
final Set<File> defs = new HashSet<>();
307309
try {
308310
if (mojo.getC() != null) {
309311
final List cSrcDirs = mojo.getC().getSourceDirectories();
@@ -364,7 +366,6 @@ public final LinkerDef getLinker(final AbstractCompileMojo mojo, final CCTask ta
364366
arg2.setValue("/MANIFESTFILE:" + task.getOutfile() + ".manifest");
365367
linker.addConfiguredLinkerArg(arg2);
366368
}
367-
368369
}
369370

370371
// Add options to linker
@@ -552,11 +553,7 @@ public final String getVersion(final AbstractNarMojo mojo) throws MojoFailureExc
552553
}
553554

554555
String version = null;
555-
String linkerPrefix = "";
556-
557-
if (this.prefix != null && (!this.prefix.isEmpty())) {
558-
linkerPrefix = this.prefix;
559-
}
556+
final String linkerPrefix = nullToEmpty(this.prefix);
560557

561558
final TextStream out = new StringTextStream();
562559
final TextStream err = new StringTextStream();
@@ -628,7 +625,7 @@ public final String getVersion(final AbstractNarMojo mojo) throws MojoFailureExc
628625
version = m.group(0);
629626
}
630627
} else {
631-
if (!this.prefix.isEmpty()) {
628+
if (!linkerPrefix.isEmpty()) {
632629
NarUtil.runCommand(linkerPrefix+this.name, new String[] {
633630
"--version"
634631
}, null, null, out, err, dbg, this.log);

‎src/main/java/com/github/maven_nar/cpptasks/CUtil.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,8 @@
2626
import org.apache.tools.ant.BuildException;
2727
import org.apache.tools.ant.Project;
2828
import org.apache.tools.ant.taskdefs.Execute;
29-
import org.apache.tools.ant.taskdefs.LogStreamHandler;
3029
import org.apache.tools.ant.types.Commandline;
3130
import org.apache.tools.ant.types.Environment;
32-
import org.apache.tools.ant.util.StringUtils;
3331

3432
/**
3533
* Some utilities used by the CC and Link tasks.
@@ -421,7 +419,8 @@ public static int runCommand(final CCTask task, final File workingDir, final Str
421419
return exe.execute();
422420
*/
423421

424-
return CommandExecution.runCommand(cmdline,workingDir,task,env.getVariablesVector());
422+
return CommandExecution.runCommand(cmdline, workingDir, task,
423+
env == null ? new Vector<Environment.Variable>() : env.getVariablesVector());
425424
} catch (final java.io.IOException exc) {
426425
throw new BuildException("Could not launch " + cmdline[0] + ": " + exc, task.getLocation());
427426
}
@@ -502,14 +501,14 @@ public static String[] toArray(final Vector src) {
502501

503502
public static String toUnixPath(final String path) {
504503
if (File.separatorChar != '/' && path.indexOf(File.separatorChar) != -1) {
505-
return StringUtils.replace(path, File.separator, "/");
504+
return path.replace(File.separator, "/");
506505
}
507506
return path;
508507
}
509508

510509
public static String toWindowsPath(final String path) {
511510
if (File.separatorChar != '\\' && path.indexOf(File.separatorChar) != -1) {
512-
return StringUtils.replace(path, File.separator, "\\");
511+
return path.replace(File.separator, "\\");
513512
}
514513
return path;
515514
}

0 commit comments

Comments
 (0)