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,153 @@
+#include <errno.h>

[pasko-google - do not use, 2011/08/15 16:23:04]

system headers first, then nacl system headers, after an empty line, then nacl
headers

[halyavin, 2011/08/16 09:05:37]

Done.

+#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));
+}
+
+/*
+ * TODO(halyavin): move to separate file because name service channel can be
+ * usefull for other APIs.

[pasko-google - do not use, 2011/08/15 16:23:04]

s/usefull/useful/

frankly, I do not see the reason for TODO here, it's so obvious

[halyavin, 2011/08/16 09:05:37]

I want to remember to do this.

+ */
+/* 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_ns_mutex(void) {
+  nacl_irt_mutex.mutex_create(&name_service_mutex);
+}
+
+int get_nameservice_channel_locked(struct NaClSrpcChannel **result) {

[pasko-google - do not use, 2011/08/15 16:23:04]

should be static

[halyavin, 2011/08/16 09:05:37]

Done.

+  int ns;
+  int connected_socket;
+  if (ns_channel_initialized) {
+    *result = &ns_channel;
+    return 0;
+  }
+  *result = 0;
+  ns = -1;
+  nacl_nameservice(&ns);
+  if (-1 == ns) {
+    print_error("Can't get name service descriptor\n");
+    return -EIO;

[pasko-google - do not use, 2011/08/15 16:23:04]

the convention is to return the *same* value as errno, so here and in other
places it should be return EIO

[halyavin, 2011/08/16 09:05:37]

Done.

+  }
+  connected_socket = imc_connect(ns);
+  if (-1 == connected_socket) {
+    print_error("Can't connect to name service\n");
+    return -EIO;
+  }
+  close(ns);
+  if (!NaClSrpcClientCtor(&ns_channel, connected_socket)) {
+    print_error("Srpc client channel ctor failed\n");
+    return -EIO;
+  }
+  *result = &ns_channel;
+  ns_channel_initialized = 1;
+  return 0;
+}
+
+/*
+ * Get name service channel.
+ * If successfull, function sets pointer to name service channel and returns 0.
+ * In case of error, function sets pointer to zero and returns error code.
+ */
+int get_nameservice_channel(struct NaClSrpcChannel **result) {

[pasko-google - do not use, 2011/08/15 16:23:04]

should be static

[halyavin, 2011/08/16 09:05:37]

This function may be extracted to the separate file in the future and so I think
it shouldn't be static.

+  int error;
+  nacl_irt_mutex.mutex_lock(name_service_mutex);
+  error = get_nameservice_channel_locked(result);
+  nacl_irt_mutex.mutex_unlock(name_service_mutex);
+  return error;
+}
+
+/*
+ * We use separate mutex so that nameservice initialization can be moved to
+ * another file. In this case name_service_mutex would not be public.
+ */
+/* Mutex to guard manifest channel initialization. */
+static int manifest_service_mutex;
+static int manifest_channel_initialized = 0;
+static struct NaClSrpcChannel manifest_channel;
+
+__attribute__((constructor)) void init_manifest_mutex(void) {
+  nacl_irt_mutex.mutex_create(&manifest_service_mutex);
+}
+
+int get_manifest_channel_locked(struct NaClSrpcChannel **result) {

[pasko-google - do not use, 2011/08/15 16:23:04]

should be static

[halyavin, 2011/08/16 09:05:37]

Done.

+  int status;
+  int manifest;
+  int manifest_conn;
+  struct NaClSrpcChannel* ns_channel;
+  if (manifest_channel_initialized) {
+    *result = &manifest_channel;
+    return 0;
+  }
+  *result = 0;
+  status = get_nameservice_channel(&ns_channel);
+  if (0 != status) {
+    return status;
+  }
+  if (NACL_SRPC_RESULT_OK != NaClSrpcInvokeBySignature(
+      ns_channel, NACL_NAME_SERVICE_LOOKUP, "ManifestNameService", O_RDWR,
+      &status, &manifest)) {
+    print_error("Nameservice lookup failed, status\n");
+    return -EIO;
+  }
+  if (-1 == manifest) {
+    print_error("Manifest descriptor is invalid\n");
+    return -EIO;
+  }
+  manifest_conn = imc_connect(manifest);
+  if (-1 == manifest_conn) {
+    print_error("Can't connect to manifest service\n");
+    return -EIO;
+  }
+  close(manifest);
+  if (!NaClSrpcClientCtor(&manifest_channel, manifest_conn)) {
+    print_error("Can't create manifest srpc channel\n");
+    return -EIO;
+  }
+  *result = &manifest_channel;
+  manifest_channel_initialized = 1;
+  return 0;
+}
+
+int get_manifest_channel(struct NaClSrpcChannel **result) {
+  int error;
+  nacl_irt_mutex.mutex_lock(manifest_service_mutex);
+  error = get_manifest_channel_locked(result);
+  nacl_irt_mutex.mutex_unlock(manifest_service_mutex);
+  return error;
+}
+
+/*
+ * Returns file descriptor or negative error code.

[pasko-google - do not use, 2011/08/15 16:23:04]

again, according to the common IRT convention, error code should return a
positive value that equals errno

[halyavin, 2011/08/16 09:05:37]

Done.

+ */
+int irt_open_resource(const char *file, int *fd) {

[pasko-google - do not use, 2011/08/15 16:23:04]

should be static

[halyavin, 2011/08/16 09:05:37]

Done.

+  int status;
+  struct NaClSrpcChannel* manifest_channel;
+  status = get_manifest_channel(&manifest_channel);
+  if (0 != status) {
+    return status;
+  }
+  *fd = 0;

[pasko-google - do not use, 2011/08/15 16:23:04]

touching a value when an error occurred is awkward, please, leave it unchanged.

[halyavin, 2011/08/16 09:05:37]

Done.

+  if (NACL_SRPC_RESULT_OK != NaClSrpcInvokeBySignature(
+      manifest_channel, NACL_NAME_SERVICE_LOOKUP, file, O_RDONLY,
+      &status, fd)) {
+    return status;
+  }
+  return 0;
+}
+
+const struct nacl_irt_resource_open nacl_irt_resource_open = {
+  irt_open_resource
+};