Extend IRT with nacl_irt_resource_open interface

src/untrusted/irt/irt_manifest.c

Index: src/untrusted/irt/irt_manifest.c
===================================================================
--- src/untrusted/irt/irt_manifest.c	(revision 0)
+++ src/untrusted/irt/irt_manifest.c	(revision 0)
@@ -0,0 +1,110 @@
+#include <nacl/nacl_srpc.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/fcntl.h>
+#include <sys/nacl_name_service.h>
+#include <sys/nacl_syscalls.h>
+
+#include "native_client/src/untrusted/irt/irt.h"
+#include "native_client/src/untrusted/irt/irt_interfaces.h"
+
+static void print_error(const char *message) {
+  write(2, message, strlen(message));
+}
+
+/* Mutex to guard name service channel initialization. */
+static int name_service_mutex;
+static int ns_channel_initialized = 0;
+static struct NaClSrpcChannel ns_channel;
+
+/* IRT is static, we don't need destructor. */
+__attribute__((constructor)) void init_mutex() {
+  nacl_irt_mutex.mutex_create(&name_service_mutex);
+}
+
+/*
+ * Returns pointer to ns_channel or 0 if lazy initialization of name service
+ * channel has failed.
+ */
+struct NaClSrpcChannel *get_nameservice_channel() {

[Roland McGrath, 2011/08/12 16:44:56]

C functions with no arguments have a prototype of (void).

[halyavin, 2011/08/15 10:35:52]

Done.

+  int ns;
+  int connected_socket;
+  struct NaClSrpcChannel *result = 0;
+  nacl_irt_mutex.mutex_lock(name_service_mutex);
+  do {

[Roland McGrath, 2011/08/12 16:44:56]

Using do {...} while (0) this way is no better than goto.
If you want it to be cleaner than goto, then put the work inside a subfunction
that's called with the lock held.

[halyavin, 2011/08/15 10:35:52]

I extracted subfunction here.

+    if (ns_channel_initialized) {
+      result = &ns_channel;
+      break;
+    }
+    ns = -1;
+    nacl_nameservice(&ns);
+    if (-1 == ns) {
+      print_error("Can't get name service descriptor\n");
+      break;
+    }
+    connected_socket = imc_connect(ns);
+    if (-1 == connected_socket) {
+      print_error("Can't connect to name service\n");
+      break;
+    }
+    close(ns);
+    if (!NaClSrpcClientCtor(&ns_channel, connected_socket)) {
+      print_error("Srpc client channel ctor failed\n");
+      break;
+    }
+    result = &ns_channel;
+    ns_channel_initialized = 1;
+  } while (0);
+  nacl_irt_mutex.mutex_unlock(name_service_mutex);
+  return result;
+}
+
+/*
+ * Returns file descriptor or -1 if some error happened.

[Roland McGrath, 2011/08/12 16:44:56]

The norm in IRT interfaces is to return an errno code or 0.
The result goes in a result parameter.  (See e.g. nacl_irt_filename.open.)

[halyavin, 2011/08/15 10:46:03]

Done.

+ */
+int irt_open_file_in_manifest(const char *file) {
+  struct NaClSrpcChannel* ns_channel;
+  int status;
+  int desc;
+  int manifest;
+  int manifest_conn;
+  struct NaClSrpcChannel manifest_channel;
+  ns_channel = get_nameservice_channel();
+  if (0 == ns_channel) {
+    return -1;
+  }
+  if (NACL_SRPC_RESULT_OK != NaClSrpcInvokeBySignature(

[Roland McGrath, 2011/08/12 16:44:56]

Why repeat this work in every call?  Shouldn't we cache the manifest connection
descriptor?

[halyavin, 2011/08/15 10:35:52]

Done.

+      ns_channel, NACL_NAME_SERVICE_LOOKUP, "ManifestNameService", O_RDWR,
+      &status, &manifest)) {
+    print_error("Nameservice lookup failed, status\n");
+    return -1;
+  }
+  if (-1 == manifest) {
+    print_error("Manifest descriptor is invalid\n");
+    return -1;
+  }
+  manifest_conn = imc_connect(manifest);
+  if (-1 == manifest_conn) {
+    print_error("Can't connect to manifest service\n");
+    return -1;
+  }
+  close(manifest);
+  if (!NaClSrpcClientCtor(&manifest_channel, manifest_conn)) {

[Roland McGrath, 2011/08/12 16:44:56]

We can also cache the manifest channel struct, no?

[halyavin, 2011/08/15 10:35:52]

Done.

+    print_error("Can't create manifest srpc channel\n");
+    return -1;
+  }
+  desc = 0;
+  if (NACL_SRPC_RESULT_OK != NaClSrpcInvokeBySignature(
+      &manifest_channel, NACL_NAME_SERVICE_LOOKUP, file, O_RDONLY,
+      &status, &desc)) {
+    print_error("Manifest lookup RPC failed\n");

[Roland McGrath, 2011/08/12 16:44:56]

This error case should not print.  The others are internal setup so printing
makes some amount of sense.  Here the failure is likely to be just that the user
supplied a bad file name.  Just return an appropriate error code.

[halyavin, 2011/08/15 10:35:52]

Done.

+    NaClSrpcDtor(&manifest_channel);
+    return -1;
+  }
+  NaClSrpcDtor(&manifest_channel);
+  return desc;
+}
+
+const struct nacl_irt_manifest_open nacl_irt_manifest_open = {
+  irt_open_file_in_manifest
+};