Remove calls to System.exit()

[samczsun]

Currently, Smali and Baksmali will call System.exit(1) on failure. This makes it difficult to integrate into another project. May I recommend using a technique like Java does, as seen in the entry point for javap:

http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/7-b147/com/sun/tools/javap/Main.java

[JesusFreke]

Fixed in 87d10da

[samczsun]

I'm loving the new API options. Just one more quick request if possible - would it be possible to add an API which allows specifying the OutputStream write to? For example, run([bak]smaliOptions, DexFile, PrintStream) or something else.

[JesusFreke]

You want to write ALL the classes to the same print stream?

Maybe some sort of print stream factory instead?

public interface BaksmaliOutput {
    PrintStream getOutputForClass(ClassDef classDef);
}

[samczsun]

Oh yes, my apologies. I wanted to generalize from my use case but it looks like I forgot that most people want to disassemble more than one file :p

[samczsun]

Quick question (and keep in mind I'm by no means an android expert at all so this might seem silly):

Any idea why I'm running into this error when calling using the API entry point even though it's fine when I run using the main(String[]) entry point?

Calling main.main(new String[] {"-o", outputDir, "-x", inputDex}); works, but I get this exception when using the code below:

Error occurred while loading boot class path files. Aborting.
org.jf.util.ExceptionWithContext: Cannot locate boot class path file /system/framework/core.jar
    at org.jf.dexlib2.analysis.ClassPath.loadClassPathEntry(ClassPath.java:277)
    at org.jf.dexlib2.analysis.ClassPath.fromClassPath(ClassPath.java:182)
    at org.jf.baksmali.baksmali.disassembleDexFile(baksmali.java:67)

            baksmaliOptions options = new baksmaliOptions();
            options.outputDirectory = tempSmali.getAbsolutePath();
            options.deodex = true;

            DexFile file = DexFileFactory.loadDexFile(tempDex, options.dexEntry, options.apiLevel, options.experimental);

            System.out.println(org.jf.baksmali.main.run(options, file));

I think I've replicated the main() method exactly, but obviously I'm missing something :P

[agorski3]

@JesusFreke: With 2.2 you seem to have backtracked on this quite a bit. There are System.exits all over the Command classes and there is no longer a static run method in the Main class that functions similarly to the main method. Moreover, setting up the options like BaksmaliOptions required to call the method disassembleDexFile is not a particularly straightforward process because of required classes like ClassPath and your setup procedure which is strewn across the command classes in methods that are not readily accessible because of the design of JCommand.

I don't really understand why you chose to use system.exit all over the code to indicate an error. Instead, I would have thrown a new single exception or a new exception containing a caught exception and let the exception propagate back to the main method. Then I would have caught it and called system.exit in main only. However, maybe you had a reason for this design.

Still, it would be great if you provided a design similar to the following for the Main class. This would allow anyone to call baksmali/smali using your checks and setup procedure from their own code without having to worry about unexpected terminations.

public static void main(String[] args) {
    try{
        run(args);
    }catch(Throwable t){
        t.printStackTrace();
        System.exit(-1);
    }
}

public static void run(String[] args){
    //No system exits beyond this point
    //The code that is in main right now
}

[JesusFreke]

I believe this should be in roughly an equivalent state as pre-2.2. The Command classes are explicitly part of the CLI, and are not intended for programmatic access. They are equivalent to the logic in the static main() function in the old CLI.

Baksmali.disassembleDexFile should be equivalent to the earlier main.run() method. And there's DumpCommand.dump as a programmatic entry point for performing an annotated dump. Both of these should be free of System.exit() calls. And now that I think about it, DumpCommand.dump should probably be moved to the Baksmali class, so it's the single programmatic entry point.

And there's ClassPathResolver to help with setting up a classpath, to help with the various complexities of finding all the dex/odex/oat files that should be on the classpath for any given case. e.g.

ClassPathResolver resolver = new ClassPathResolver(Lists.newArrayList("."), Lists.<String>newArrayList(), dexFile, 24);
ClassPath classPath = new ClassPath(resolver.getResolvedClassProviders(), true, 79);

I guess there's the various list commands that don't have a good programmatic entry point, but those are all pretty simple, and for programmatic access, you're probably better off just iterating over the various lists yourself.

[agorski3]

I suppose you are correct. Still it would be nice to not have to reimplement setup procedures that already exist in your code but are unusable because they are hidden behind JCommand and riddled with system.exits. Especially since you will likely modify said setup procedures in the future leading to a disjoint in the reimplemented setup procedures and your setup procedures which could cause issues.

Also, you never mentioned your reasoning behind using system.exit. If you had not guessed I tend to hate the use of it design wise as it can lead to all sorts of annoying issues.

[Lanchon]

hey JF, not that i particularly care about this (though i see the point), but you can accommodate this very easily:

public static void main(String[] args) {
    try {
        run(args);
    } catch (DoSystemExit e) {
        System.exit(e.getExitCode());
    }
}

then search and replace all System.ext(n) with throw new DoSystemExit(n).

DoSystemExit extends Error so you don't need to declare it and you won't catch it if your code does the catch (Exception e) thingy all over.

of course you will catch it if you catch (Throwable t) but you should never do that! well unless you are doing threading stuff and you want to transfer an exception to a different thread. (even so, before rethrowing a 't' wrapped in a new WorkerThreadException(t) you should check instanceof Error and throw the non-wrapped t in that case.)

and the other difference is that finally {} blocks will be invoked, which under almost any setting should be a good thing.

[Lanchon]

actually, this is much better:

public static void main(String[] args) {
    System.exit(run(args));
}

public static int run(String[] args) {
    try {
        privateRun(args);
        return 0;
    } catch (DoSystemExit e) {
        return e.getExitCode();
    }
}