NaCl: Expose NaClApp to embedding layer.

src/trusted/service_runtime/sel_main_chrome.c

Index: src/trusted/service_runtime/sel_main_chrome.c
diff --git a/src/trusted/service_runtime/sel_main_chrome.c b/src/trusted/service_runtime/sel_main_chrome.c
index 89e7f3054dfd8633ee286574fa99ab0aee09c9d2..47774ab6df0521ce1851aae702945dac2ce0a2a1 100644
--- a/src/trusted/service_runtime/sel_main_chrome.c
+++ b/src/trusted/service_runtime/sel_main_chrome.c
@@ -124,12 +124,24 @@ static void NaClLoadIrt(struct NaClApp *nap, int irt_fd) {
   NaClDescUnref(nd);
 }
 
-void NaClChromeMainStart(struct NaClChromeMainArgs *args) {
+struct NaClApp* NaClAppCreate(void) {

[Mark Seaborn, 2014/01/17 16:42:06]

Spacing style should be " *" in NaCl-side code (same elsewhere in this change).

+  struct NaClApp* nap = (struct NaClApp*)malloc(sizeof(struct NaClApp));
+  if (!nap)

[Mark Seaborn, 2014/01/17 16:42:06]

NaCl style is "nap == NULL"

+    return NULL;
+
+  memset(nap, 0, sizeof(struct NaClApp));
+  if (NaClAppCtor(nap) != 0) {

[Mark Seaborn, 2014/01/17 16:42:06]

There's a correctness problem here.  If NaClAppCreate() is called before
NaClChromeMainStartApp(), the former will happen without NaClAllModulesInit()
having been called.

Can you run trybots for NaCl changes before sending them for review, please?  I
don't know if this works with sel_main_chrome_test on all platforms.

BTW, "git cl try" only works for changes to the Chromium repo, not any other
repo.  You have to use "git try" instead.

+    free(nap);
+    return NULL;
+  }
+  return nap;
+}
+
+void NaClChromeMainStartApp(struct NaClApp *nap,
+                            struct NaClChromeMainArgs *args) {
   char *av[1];
   int ac = 1;
   const char **envp;
-  struct NaClApp state;
-  struct NaClApp *nap = &state;
   NaClErrorCode errcode = LOAD_INTERNAL;
   int ret_code = 1;
   struct NaClEnvCleanser env_cleanser;
@@ -155,19 +167,13 @@ void NaClChromeMainStart(struct NaClChromeMainArgs *args) {
    * Clear state so that NaClBootstrapChannelErrorReporter will be
    * able to know if the bootstrap channel is available or not.
    */
-  memset(&state, 0, sizeof state);
   NaClAllModulesInit();
   NaClBootstrapChannelErrorReporterInit();
-  NaClErrorLogHookInit(NaClBootstrapChannelErrorReporter, &state);
+  NaClErrorLogHookInit(NaClBootstrapChannelErrorReporter, nap);
 
   /* to be passed to NaClMain, eventually... */
   av[0] = "NaClMain";
 
-  if (NACL_FI_ERROR_COND("AppCtor", !NaClAppCtor(&state))) {
-    NaClLog(LOG_FATAL, "Error while constructing app state\n");
-    goto done;
-  }
-
   errcode = LOAD_OK;
 
   /* Allow or disallow dyncode API based on args. */
@@ -269,7 +275,7 @@ void NaClChromeMainStart(struct NaClChromeMainArgs *args) {
 #endif
 
   /* Give debuggers a well known point at which xlate_base is known.  */
-  NaClGdbHook(&state);
+  NaClGdbHook(nap);
 
   NaClCreateServiceSocket(nap);
   /*
@@ -406,3 +412,11 @@ void NaClChromeMainStart(struct NaClChromeMainArgs *args) {
 
   NaClExit(ret_code);
 }
+
+void NaClChromeMainStart(struct NaClChromeMainArgs *args) {
+  struct NaClApp *nap = NaClAppCreate();

[dmichael (off chromium), 2014/01/16 21:30:26]

Is there a need to do it on the heap here? Could you just do it on the stack in
this case, and get rid of NaClAppCreate?

[dmichael (off chromium), 2014/01/17 16:00:46]

Okay, we just chatted. Since:
1) This code is going to be deleted soon anyway
and
2) Using NaClAppCreate here exercises that function (whereas a stack variable
wouldn't)
...
the code's good as you have it. I think a TODO to delete this function wouldn't
hurt.

+  if (!nap) {
+    NaClLog(LOG_FATAL, "NaClChromeMainStart: Could not create NaClApp.\n");
+  }
+  NaClChromeMainStartApp(nap, args);
+}