Refactor the process of choosing validators.

src/trusted/validator/validator_init.c

+/*
+ * Copyright (c) 2012 The Native Client Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "native_client/src/trusted/validator/ncvalidate.h"
+
+int NaClUseDfaValidator() {
+  if (getenv("NACL_DANGEROUS_USE_DFA_VALIDATOR") != NULL) {
+    return 1;
+  }
+  return 0;
+}
+
+const struct NaClValidatorInterface* NaClCreateValidator() {
+#if defined(__arm__)
+  return NaClValidatorCreateArm();
+#elif NACL_ARCH(NACL_TARGET_ARCH) != NACL_x86
+#error("No validator available for this architecture!")
+#elif NACL_TARGET_SUBARCH == 32 && defined(NACL_STANDALONE)

[Nick Bray, 2012/05/22 23:46:31]

I don't like splitting the ARCH/SUBARCH check - it seems fragile.  The order of
the checks now matter.  Why not stick with the previous implementation?

[pasko-google - do not use, 2012/05/23 14:28:11]

huh, that was funny. The macro __i386__ is not defined on Windows, so I had to
either put windows macro fun here which spans more than one line, or use the
macros we already have for this.

Additionally there were some problems on arm, I do not remember what
specifically, but I can recheck if you want.

[Nick Bray, 2012/05/24 01:17:15]

OK, I see why you split the ARCH from the SUBARCH now that I looked everywhere
in our code base - it's just the way things are done.  So silly. :/  The
sensitivity to statement order, however, sill strikes me as begging for bugs. 
It looks like we have our choice of ugly looking code.

1) Leave it as is.
2) Nestled preprocessor blocks (indented?)
3) Decompose into multiple functions (ARM, x86, possibally x86-32 and x86-64),
surrounded by preprocessor switches.  This may or may not make 2) more readable.
4) Live with > 80 character lines.

Getting the formatting right may be more hassle than it's worth, but the small
landmine worries me.  The linkage issues are more important to me, your call.

[pasko-google - do not use, 2012/05/24 10:16:57]

When writing I thought you'd like this because it is written in the style you
suggested with an exception in a couple of lines. But well, if you do not like
it overall I would better switch to (2).
I prefer this, indented in the preprocessor lines only. That is a common
practice that I used to accept. When code is short. If the code gets longer, we
will think again.
This would make the code less localized, longer, harder to make sure that
specific variants are mutually exclusive.
please no, the question is not in length, but in the number of conditions that
have to be coded in a single line

+  if (NaClUseDfaValidator()) {
+    fprintf(stderr, "DANGER! USING THE EXPERIMENTAL DFA VALIDATOR!\n");
+    return NaClDfaValidatorCreate_x86_32();
+  } else {
+    return NaClValidatorCreate_x86_32();
+  }
+#elif NACL_TARGET_SUBARCH == 32
+  return NaClValidatorCreate_x86_32();
+#elif NACL_TARGET_SUBARCH == 64 && defined(NACL_STANDALONE)
+  if (NaClUseDfaValidator()) {
+    fprintf(stderr, "DANGER! USING THE EXPERIMENTAL DFA VALIDATOR!\n");
+    return NaClDfaValidatorCreate_x86_64();
+  } else {
+    return NaClValidatorCreate_x86_64();
+  }
+#elif NACL_TARGET_SUBARCH == 64
+  return NaClValidatorCreate_x86_64();
+#endif
+}