Skip to contentSkip to conversation

Microsoft Visual C patches

PATCH In review Andrei Errapart 4 participants 13 messages Jun 15, 2014 – Jun 20, 2014 6 messages not in the archive

lore
Share & tools
lore

Andrei Errapart

author
#0 lore raw

PATCH 1/1 Microsoft Visual C patches

Hallo,

The attached patches are the result of compiling the dtc with the Microsoft Visual C (MSVC) in order to be able to run the dtc under Microsoft Windows.

Short descriptions of the patches:
checks: MSVC is unable to handle 0-sized array literals.
fopen: Binary files should be opened in binary mode.
signed-char: For MSVC, char defaults to signed char.
dtc-header:
1) Variadic macro in the form "args..." is a GCC extension.
2) MSVC requires 0 to be present in a struct literal.

On a Linux, all tests pass when these patches have been applied.

We have been using the dtc on Windows computers for a few days and haven't had any problems so far. In the case people are interested in having the MSVC project file(s) and the few lightweight compatibility functions required for compilation, let me know and I can clean them up for public use.

best regards,
Andrei

checks.c +27 -25
index 47eda65..cf07864 100644
@@ -61,17 +61,19 @@ struct check {
 #define CHECK_ENTRY(nm, tfn, nfn, pfn, d, w, e, ...)	       \
 	static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
 	static struct check nm = { \
-		.name = #nm, \
-		.tree_fn = (tfn), \
-		.node_fn = (nfn), \
-		.prop_fn = (pfn), \
-		.data = (d), \
-		.warn = (w), \
-		.error = (e), \
-		.status = UNCHECKED, \
-		.num_prereqs = ARRAY_SIZE(nm##_prereqs), \
-		.prereq = nm##_prereqs, \
+		#nm, \
+		(tfn), \
+		(nfn), \
+		(pfn), \
+		(d), \
+		(w), \
+		(e), \
+		UNCHECKED, \
+		false, \
+		ARRAY_SIZE(nm##_prereqs), \
+		nm##_prereqs \
 	};
+
 #define WARNING(nm, tfn, nfn, pfn, d, ...) \
 	CHECK_ENTRY(nm, tfn, nfn, pfn, d, true, false, __VA_ARGS__)
 #define ERROR(nm, tfn, nfn, pfn, d, ...) \

@@ -153,7 +155,7 @@ static bool run_check(struct check *c, struct node *dt)
 
 	c->inprogress = true;
 
-	for (i = 0; i < c->num_prereqs; i++) {
+	for (i = 0; i < c->num_prereqs && c->prereq[i]!=NULL; i++) {
 		struct check *prq = c->prereq[i];
 		error = error || run_check(prq, dt);
 		if (prq->status != PASSED) {

@@ -192,7 +194,7 @@ static inline void check_always_fail(struct check *c, struct node *dt)
 {
 	FAIL(c, "always_fail check");
 }
-TREE_CHECK(always_fail, NULL);
+TREE_CHECK(always_fail, NULL, 0);
 
 static void check_is_string(struct check *c, struct node *root,
 			    struct node *node)

@@ -209,9 +211,9 @@ static void check_is_string(struct check *c, struct node *root,
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_STRING(nm, propname) \
-	WARNING(nm, NULL, check_is_string, NULL, (propname))
+	WARNING(nm, NULL, check_is_string, NULL, (propname), 0)
 #define ERROR_IF_NOT_STRING(nm, propname) \
-	ERROR(nm, NULL, check_is_string, NULL, (propname))
+	ERROR(nm, NULL, check_is_string, NULL, (propname), 0)
 
 static void check_is_cell(struct check *c, struct node *root,
 			  struct node *node)

@@ -228,9 +230,9 @@ static void check_is_cell(struct check *c, struct node *root,
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_CELL(nm, propname) \
-	WARNING(nm, NULL, check_is_cell, NULL, (propname))
+	WARNING(nm, NULL, check_is_cell, NULL, (propname), 0)
 #define ERROR_IF_NOT_CELL(nm, propname) \
-	ERROR(nm, NULL, check_is_cell, NULL, (propname))
+	ERROR(nm, NULL, check_is_cell, NULL, (propname), 0)
 
 /*
  * Structural check functions

@@ -249,7 +251,7 @@ static void check_duplicate_node_names(struct check *c, struct node *dt,
 				FAIL(c, "Duplicate node name %s",
 				     child->fullpath);
 }
-NODE_ERROR(duplicate_node_names, NULL);
+NODE_ERROR(duplicate_node_names, NULL, 0);
 
 static void check_duplicate_property_names(struct check *c, struct node *dt,
 					   struct node *node)

@@ -266,7 +268,7 @@ static void check_duplicate_property_names(struct check *c, struct node *dt,
 		}
 	}
 }
-NODE_ERROR(duplicate_property_names, NULL);
+NODE_ERROR(duplicate_property_names, NULL, 0);
 
 #define LOWERCASE	"abcdefghijklmnopqrstuvwxyz"
 #define UPPERCASE	"ABCDEFGHIJKLMNOPQRSTUVWXYZ"

@@ -282,7 +284,7 @@ static void check_node_name_chars(struct check *c, struct node *dt,
 		FAIL(c, "Bad character '%c' in node %s",
 		     node->name[n], node->fullpath);
 }
-NODE_ERROR(node_name_chars, PROPNODECHARS "@");
+NODE_ERROR(node_name_chars, PROPNODECHARS "@", 0);
 
 static void check_node_name_format(struct check *c, struct node *dt,
 				   struct node *node)

@@ -302,7 +304,7 @@ static void check_property_name_chars(struct check *c, struct node *dt,
 		FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
 		     prop->name[n], prop->name, node->fullpath);
 }
-PROP_ERROR(property_name_chars, PROPNODECHARS);
+PROP_ERROR(property_name_chars, PROPNODECHARS, 0);
 
 #define DESCLABEL_FMT	"%s%s%s%s%s"
 #define DESCLABEL_ARGS(node,prop,mark)		\

@@ -358,7 +360,7 @@ static void check_duplicate_label_prop(struct check *c, struct node *dt,
 		check_duplicate_label(c, dt, m->ref, node, prop, m);
 }
 ERROR(duplicate_label, NULL, check_duplicate_label_node,
-      check_duplicate_label_prop, NULL);
+      check_duplicate_label_prop, NULL, 0);
 
 static void check_explicit_phandles(struct check *c, struct node *root,
 				    struct node *node, struct property *prop)

@@ -417,7 +419,7 @@ static void check_explicit_phandles(struct check *c, struct node *root,
 
 	node->phandle = phandle;
 }
-PROP_ERROR(explicit_phandles, NULL);
+PROP_ERROR(explicit_phandles, NULL, 0);
 
 static void check_name_properties(struct check *c, struct node *root,
 				  struct node *node)

@@ -649,7 +651,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
 		     "property");
 }
-TREE_WARNING(obsolete_chosen_interrupt_controller, NULL);
+TREE_WARNING(obsolete_chosen_interrupt_controller, NULL, 0);
 
 static struct check *check_table[] = {
 	&duplicate_node_names, &duplicate_property_names,

@@ -678,7 +680,7 @@ static void enable_warning_error(struct check *c, bool warn, bool error)
 
 	/* Raising level, also raise it for prereqs */
 	if ((warn && !c->warn) || (error && !c->error))
-		for (i = 0; i < c->num_prereqs; i++)
+		for (i = 0; i < c->num_prereqs && c->prereq[i] != NULL; i++)
 			enable_warning_error(c->prereq[i], warn, error);
 
 	c->warn = c->warn || warn;

@@ -696,7 +698,7 @@ static void disable_warning_error(struct check *c, bool warn, bool error)
 			struct check *cc = check_table[i];
 			int j;
 
-			for (j = 0; j < cc->num_prereqs; j++)
+			for (j = 0; j < cc->num_prereqs && cc->prereq[j] != NULL; j++)
 				if (cc->prereq[j] == c)
 					disable_warning_error(cc, warn, error);
 		}
fstree.c +1 -1
index 4d2791c..6d1beec 100644
@@ -52,7 +52,7 @@ static struct node *read_fstree(const char *dirname)
 			struct property *prop;
 			FILE *pfile;
 
-			pfile = fopen(tmpname, "r");
+			pfile = fopen(tmpname, "rb");
 			if (! pfile) {
 				fprintf(stderr,
 					"WARNING: Cannot open %s: %s\n",
srcpos.c +1 -1
index 4549773..f534c22 100644
@@ -77,7 +77,7 @@ static char *try_open(const char *dirname, const char *fname, FILE **fp)
 	else
 		fullname = join_path(dirname, fname);
 
-	*fp = fopen(fullname, "r");
+	*fp = fopen(fullname, "rb");
 	if (!*fp) {
 		free(fullname);
 		fullname = NULL;
dtc.c +1 -1
index d36ccdc..e3665b6 100644
@@ -237,7 +237,7 @@ int main(int argc, char *argv[])
 	if (streq(outname, "-")) {
 		outf = stdout;
 	} else {
-		outf = fopen(outname, "w");
+		outf = fopen(outname, "wb");
 		if (! outf)
 			die("Couldn't open output file %s: %s\n",
 			    outname, strerror(errno));
treesource.c +1 -1
index bf7a626..2386b93 100644
@@ -178,7 +178,7 @@ static void write_propval_bytes(FILE *f, struct data val)
 			m = m->next;
 		}
 
-		fprintf(f, "%02hhx", *bp++);
+		fprintf(f, "%02hhx", (unsigned char)(*bp++));
 		if ((const void *)bp >= propend)
 			break;
 		fprintf(f, " ");
dtc.h +3 -4
index 20de073..a9d1775 100644
@@ -38,9 +38,9 @@
 #include "util.h"
 
 #ifdef DEBUG
-#define debug(fmt,args...)	printf(fmt, ##args)
+#define debug(...)		printf(__VA_ARGS__)
 #else
-#define debug(fmt,args...)
+#define debug(...)
 #endif
 
 

@@ -87,8 +87,7 @@ struct data {
 	struct marker *markers;
 };
 
-
-#define empty_data ((struct data){ /* all .members = 0 or NULL */ })
+#define empty_data ((struct data){ 0 /* all .members = 0 or NULL */ })
 
 #define for_each_marker(m) \
 	for (; (m); (m) = (m)->next)

A message that is not in the archive (sent off-list or not yet received): 539DBFC8.4050204-VvktuG2w+SIZux3j3Bed6fC9HSW9iNxf@public.gmane.org

#2 lore raw
Quoted 6 lines · Andrei Errapart

Hallo,

The attached patches are the result of compiling the dtc with the Microsoft Visual C (MSVC) in order to be able to run the dtc under Microsoft Windows.

Ok, so, in order to apply these, I'll need them one by one, with their own commit messages and Signed-off-by lines.

Quoted 40 lines

Short descriptions of the patches:
checks: MSVC is unable to handle 0-sized array literals.
fopen: Binary files should be opened in binary mode.
signed-char: For MSVC, char defaults to signed char.
dtc-header:
1) Variadic macro in the form "args..." is a GCC extension.
2) MSVC requires 0 to be present in a struct literal.

On a Linux, all tests pass when these patches have been applied.

We have been using the dtc on Windows computers for a few days and haven't had any problems so far. In the case people are interested in having the MSVC project file(s) and the few lightweight compatibility functions required for compilation, let me know and I can clean them up for public use.

best regards,
Andrei

checks.c +1 -10
index 47eda65..cf07864 100644
@@ -61,17 +61,19 @@ struct check {
 #define CHECK_ENTRY(nm, tfn, nfn, pfn, d, w, e, ...)	       \
 	static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
 	static struct check nm = { \
-		.name = #nm, \
-		.tree_fn = (tfn), \
-		.node_fn = (nfn), \
-		.prop_fn = (pfn), \
-		.data = (d), \
-		.warn = (w), \
-		.error = (e), \
-		.status = UNCHECKED, \
-		.num_prereqs = ARRAY_SIZE(nm##_prereqs), \
-		.prereq = nm##_prereqs, \
+		#nm, \

checks.c struct check { · line 74

Quoted 10 lines
checks.c +10 -0
+		(tfn), \
+		(nfn), \
+		(pfn), \
+		(d), \
+		(w), \
+		(e), \
+		UNCHECKED, \
+		false, \
+		ARRAY_SIZE(nm##_prereqs), \
+		nm##_prereqs \

Um.. I don't see what removing the C99 initializers has to do with MSVC not supporting 0 size arrays

checks.c struct check { · line 76

Quoted 2 lines
checks.c +1 -0
 	};
+

Please don't include unrelated whitespace changes.

checks.c static bool run_check(struct check *c, struct node *dt) · line 158

Quoted 9 lines
checks.c +1 -1
 #define WARNING(nm, tfn, nfn, pfn, d, ...) \
 	CHECK_ENTRY(nm, tfn, nfn, pfn, d, true, false, __VA_ARGS__)
 #define ERROR(nm, tfn, nfn, pfn, d, ...) \

@@ -153,7 +155,7 @@ static bool run_check(struct check *c, struct node *dt)
 
 	c->inprogress = true;
 
-	for (i = 0; i < c->num_prereqs; i++) {
+	for (i = 0; i < c->num_prereqs && c->prereq[i]!=NULL; i++) {

Hrm. I think I see what you're doing here, but it's kinda subtle. Without a comment, I think someone's very likely to take it out again.

[snip]

Quoted 31 lines
fstree.c +1 -1
index 4d2791c..6d1beec 100644
@@ -52,7 +52,7 @@ static struct node *read_fstree(const char *dirname)
 			struct property *prop;
 			FILE *pfile;
 
-			pfile = fopen(tmpname, "r");
+			pfile = fopen(tmpname, "rb");
 			if (! pfile) {
 				fprintf(stderr,
 					"WARNING: Cannot open %s: %s\n",
srcpos.c +1 -1
index 4549773..f534c22 100644
@@ -77,7 +77,7 @@ static char *try_open(const char *dirname, const char *fname, FILE **fp)
 	else
 		fullname = join_path(dirname, fname);
 
-	*fp = fopen(fullname, "r");
+	*fp = fopen(fullname, "rb");
 	if (!*fp) {
 		free(fullname);
 		fullname = NULL;
dtc.c +0 -0
index d36ccdc..e3665b6 100644

dtc.c int main(int argc, char *argv[]) · line 243

Quoted 8 lines
dtc.c +1 -1
@@ -237,7 +237,7 @@ int main(int argc, char *argv[])
 	if (streq(outname, "-")) {
 		outf = stdout;
 	} else {
-		outf = fopen(outname, "w");
+		outf = fopen(outname, "wb");
 		if (! outf)
 			die("Couldn't open output file %s: %s\n",
 			    outname, strerror(errno));

The fopen() mode patch I'm happy to take once it's sent with its own comment and signed-off-by lines.

treesource.c static void write_propval_bytes(FILE *f, struct data val) · line 184

Quoted 13 lines
treesource.c +1 -1
index bf7a626..2386b93 100644
@@ -178,7 +178,7 @@ static void write_propval_bytes(FILE *f, struct data val)
 			m = m->next;
 		}
 
-		fprintf(f, "%02hhx", *bp++);
+		fprintf(f, "%02hhx", (unsigned char)(*bp++));
 		if ((const void *)bp >= propend)
 			break;
 		fprintf(f, " ");

Likewise the unsigned char patch.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

Hi Andrei,

Your patches are very timely for us. We were wondering how to compile dtc for Windows. We look forward to your resubmissions being accepted.

Thanks,

Matthew Gerlach

Quoted 144 lines · David Gibson
Quoted 6 lines · Andrei Errapart

Hallo,

The attached patches are the result of compiling the dtc with the Microsoft Visual C (MSVC) in order to be able to run the dtc under Microsoft Windows.

Ok, so, in order to apply these, I'll need them one by one, with their own commit messages and Signed-off-by lines.

Quoted 40 lines

Short descriptions of the patches:

checks:               MSVC is unable to handle 0-sized array literals.
fopen:                Binary files should be opened in binary mode.
signed-char:  For MSVC, char defaults to signed char.

dtc-header:
1) Variadic macro in the form "args..." is a GCC extension.
2) MSVC requires 0 to be present in a struct literal.

On a Linux, all tests pass when these patches have been applied.

We have been using the dtc on Windows computers for a few days and haven't had any problems so far. In the case people are interested in having the MSVC project file(s) and the few lightweight compatibility functions required for compilation, let me know and I can clean them up for public use.

best regards,
Andrei

checks.c +1 -10
index 47eda65..cf07864 100644
@@ -61,17 +61,19 @@ struct check {
 #define CHECK_ENTRY(nm, tfn, nfn, pfn, d, w, e, ...)        \
      static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
      static struct check nm = { \
-             .name = #nm, \
-             .tree_fn = (tfn), \
-             .node_fn = (nfn), \
-             .prop_fn = (pfn), \
-             .data = (d), \
-             .warn = (w), \
-             .error = (e), \
-             .status = UNCHECKED, \
-             .num_prereqs = ARRAY_SIZE(nm##_prereqs), \
-             .prereq = nm##_prereqs, \
+             #nm, \

checks.c struct check { · line 74

Quoted 10 lines
checks.c +10 -0
+             (tfn), \
+             (nfn), \
+             (pfn), \
+             (d), \
+             (w), \
+             (e), \
+             UNCHECKED, \
+             false, \
+             ARRAY_SIZE(nm##_prereqs), \
+             nm##_prereqs \

Um.. I don't see what removing the C99 initializers has to do with MSVC not supporting 0 size arrays

checks.c struct check { · line 76

Quoted 2 lines
checks.c +1 -0
      };
+

Please don't include unrelated whitespace changes.

checks.c static bool run_check(struct check *c, struct node *dt) · line 158

Quoted 9 lines
checks.c +1 -1
 #define WARNING(nm, tfn, nfn, pfn, d, ...) \
      CHECK_ENTRY(nm, tfn, nfn, pfn, d, true, false, __VA_ARGS__)
 #define ERROR(nm, tfn, nfn, pfn, d, ...) \

@@ -153,7 +155,7 @@ static bool run_check(struct check *c, struct node *dt)
 
      c->inprogress = true;
 
-     for (i = 0; i < c->num_prereqs; i++) {
+     for (i = 0; i < c->num_prereqs && c->prereq[i]!=NULL; i++) {

Hrm. I think I see what you're doing here, but it's kinda subtle. Without a comment, I think someone's very likely to take it out again.

[snip]

Quoted 31 lines
fstree.c +1 -1
index 4d2791c..6d1beec 100644
@@ -52,7 +52,7 @@ static struct node *read_fstree(const char *dirname)
                      struct property *prop;
                      FILE *pfile;
 
-                     pfile = fopen(tmpname, "r");
+                     pfile = fopen(tmpname, "rb");
                      if (! pfile) {
                              fprintf(stderr,
                                      "WARNING: Cannot open %s: %s\n",
srcpos.c +1 -1
index 4549773..f534c22 100644
@@ -77,7 +77,7 @@ static char *try_open(const char *dirname, const char *fname, FILE **fp)
      else
              fullname = join_path(dirname, fname);
 
-     *fp = fopen(fullname, "r");
+     *fp = fopen(fullname, "rb");
      if (!*fp) {
              free(fullname);
              fullname = NULL;
dtc.c +0 -0
index d36ccdc..e3665b6 100644

dtc.c int main(int argc, char *argv[]) · line 243

Quoted 8 lines
dtc.c +1 -1
@@ -237,7 +237,7 @@ int main(int argc, char *argv[])
      if (streq(outname, "-")) {
              outf = stdout;
      } else {
-             outf = fopen(outname, "w");
+             outf = fopen(outname, "wb");
              if (! outf)
                      die("Couldn't open output file %s: %s\n",
                          outname, strerror(errno));

The fopen() mode patch I'm happy to take once it's sent with its own comment and signed-off-by lines.

treesource.c static void write_propval_bytes(FILE *f, struct data val) · line 184

Quoted 13 lines
treesource.c +1 -1
index bf7a626..2386b93 100644
@@ -178,7 +178,7 @@ static void write_propval_bytes(FILE *f, struct data val)
                      m = m->next;
              }
 
-             fprintf(f, "%02hhx", *bp++);
+             fprintf(f, "%02hhx", (unsigned char)(*bp++));
              if ((const void *)bp >= propend)
                      break;
              fprintf(f, " ");

Likewise the unsigned char patch.

Quoted 5 lines

--

David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!

http://www.ozlabs.org/~dgibson

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#5 lore raw

PATCH 1/4 Microsoft Visual C patches

In reply to a message not in the archive

commit b8ad167d89d7f5ba6398ffeab6a7f9ece954f208

Author: Andrei Errapart <[email protected]>
Date:   Wed Jun 18 18:43:17 2014 +0200

* MSVC doesn't support 0-sized arrays.
* Initialize check->inprogress, too.

Signed-off-by Andrei Errapart <[email protected]>
checks.c +20 -15
index 47eda65..bc3c259 100644
@@ -69,6 +69,7 @@ struct check {
 		.warn = (w), \
 		.error = (e), \
 		.status = UNCHECKED, \
+		.inprogress = false, \
 		.num_prereqs = ARRAY_SIZE(nm##_prereqs), \
 		.prereq = nm##_prereqs, \
 	};

@@ -153,7 +154,7 @@ static bool run_check(struct check *c, struct node *dt)
 
 	c->inprogress = true;
 
-	for (i = 0; i < c->num_prereqs; i++) {
+	for (i = 0; i < c->num_prereqs && c->prereq[i]!=NULL; i++) {
 		struct check *prq = c->prereq[i];
 		error = error || run_check(prq, dt);
 		if (prq->status != PASSED) {

@@ -192,7 +193,7 @@ static inline void check_always_fail(struct check *c, struct node *dt)
 {
 	FAIL(c, "always_fail check");
 }
-TREE_CHECK(always_fail, NULL);
+TREE_CHECK(always_fail, NULL, 0);
 
 static void check_is_string(struct check *c, struct node *root,
 			    struct node *node)

@@ -209,9 +210,9 @@ static void check_is_string(struct check *c, struct node *root,
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_STRING(nm, propname) \
-	WARNING(nm, NULL, check_is_string, NULL, (propname))
+	WARNING(nm, NULL, check_is_string, NULL, (propname), 0)
 #define ERROR_IF_NOT_STRING(nm, propname) \
-	ERROR(nm, NULL, check_is_string, NULL, (propname))
+	ERROR(nm, NULL, check_is_string, NULL, (propname), 0)
 
 static void check_is_cell(struct check *c, struct node *root,
 			  struct node *node)

@@ -228,9 +229,9 @@ static void check_is_cell(struct check *c, struct node *root,
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_CELL(nm, propname) \
-	WARNING(nm, NULL, check_is_cell, NULL, (propname))
+	WARNING(nm, NULL, check_is_cell, NULL, (propname), 0)
 #define ERROR_IF_NOT_CELL(nm, propname) \
-	ERROR(nm, NULL, check_is_cell, NULL, (propname))
+	ERROR(nm, NULL, check_is_cell, NULL, (propname), 0)
 
 /*
  * Structural check functions

@@ -249,7 +250,7 @@ static void check_duplicate_node_names(struct check *c, struct node *dt,
 				FAIL(c, "Duplicate node name %s",
 				     child->fullpath);
 }
-NODE_ERROR(duplicate_node_names, NULL);
+NODE_ERROR(duplicate_node_names, NULL, 0);
 
 static void check_duplicate_property_names(struct check *c, struct node *dt,
 					   struct node *node)

@@ -266,7 +267,7 @@ static void check_duplicate_property_names(struct check *c, struct node *dt,
 		}
 	}
 }
-NODE_ERROR(duplicate_property_names, NULL);
+NODE_ERROR(duplicate_property_names, NULL, 0);
 
 #define LOWERCASE	"abcdefghijklmnopqrstuvwxyz"
 #define UPPERCASE	"ABCDEFGHIJKLMNOPQRSTUVWXYZ"

@@ -282,7 +283,7 @@ static void check_node_name_chars(struct check *c, struct node *dt,
 		FAIL(c, "Bad character '%c' in node %s",
 		     node->name[n], node->fullpath);
 }
-NODE_ERROR(node_name_chars, PROPNODECHARS "@");
+NODE_ERROR(node_name_chars, PROPNODECHARS "@", 0);
 
 static void check_node_name_format(struct check *c, struct node *dt,
 				   struct node *node)

@@ -302,7 +303,7 @@ static void check_property_name_chars(struct check *c, struct node *dt,
 		FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
 		     prop->name[n], prop->name, node->fullpath);
 }
-PROP_ERROR(property_name_chars, PROPNODECHARS);
+PROP_ERROR(property_name_chars, PROPNODECHARS, 0);
 
 #define DESCLABEL_FMT	"%s%s%s%s%s"
 #define DESCLABEL_ARGS(node,prop,mark)		\

@@ -358,7 +359,7 @@ static void check_duplicate_label_prop(struct check *c, struct node *dt,
 		check_duplicate_label(c, dt, m->ref, node, prop, m);
 }
 ERROR(duplicate_label, NULL, check_duplicate_label_node,
-      check_duplicate_label_prop, NULL);
+      check_duplicate_label_prop, NULL, 0);
 
 static void check_explicit_phandles(struct check *c, struct node *root,
 				    struct node *node, struct property *prop)

@@ -417,7 +418,7 @@ static void check_explicit_phandles(struct check *c, struct node *root,
 
 	node->phandle = phandle;
 }
-PROP_ERROR(explicit_phandles, NULL);
+PROP_ERROR(explicit_phandles, NULL, 0);
 
 static void check_name_properties(struct check *c, struct node *root,
 				  struct node *node)

@@ -649,7 +650,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
 		FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
 		     "property");
 }
-TREE_WARNING(obsolete_chosen_interrupt_controller, NULL);
+TREE_WARNING(obsolete_chosen_interrupt_controller, NULL, 0);
 
 static struct check *check_table[] = {
 	&duplicate_node_names, &duplicate_property_names,

@@ -678,8 +679,10 @@ static void enable_warning_error(struct check *c, bool warn, bool error)
 
 	/* Raising level, also raise it for prereqs */
 	if ((warn && !c->warn) || (error && !c->error))
-		for (i = 0; i < c->num_prereqs; i++)
+		for (i = 0; i < c->num_prereqs && c->prereq[i] != NULL; i++)
 			enable_warning_error(c->prereq[i], warn, error);
+		/* check->prereq[i] might be NULL; this is because MSVC doesn't
+		 * support 0-sized arrays.  */
 
 	c->warn = c->warn || warn;
 	c->error = c->error || error;

@@ -696,9 +699,11 @@ static void disable_warning_error(struct check *c, bool warn, bool error)
 			struct check *cc = check_table[i];
 			int j;
 
-			for (j = 0; j < cc->num_prereqs; j++)
+			for (j = 0; j < cc->num_prereqs && cc->prereq[j] != NULL; j++)
 				if (cc->prereq[j] == c)
 					disable_warning_error(cc, warn, error);
+			/* check->prereq[j] might be NULL; this is because MSVC doesn't
+			 * support 0-sized arrays.  */
 		}
 	}

A message that is not in the archive (sent off-list or not yet received): 53A1C5FC.9040105-VvktuG2w+SIZux3j3Bed6fC9HSW9iNxf@public.gmane.org

#7 lore raw
Quoted 7 lines · Andrei Errapart

commit b8ad167d89d7f5ba6398ffeab6a7f9ece954f208

Author: Andrei Errapart <[email protected]>
Date:   Wed Jun 18 18:43:17 2014 +0200

* MSVC doesn't support 0-sized arrays.

Ugh, this is by far the ugliest of these workarounds, and the reason for it in the code is really non-obvious. I'll have to think about this some more.

Quoted 1 line

* Initialize check->inprogress, too.

Why..?

Quoted 1 line
Signed-off-by Andrei Errapart <[email protected]>

I've applied patches 2, 3, and 4. But for future reference, please remember:

* Patches should be inline, not attachments
* Commit message and signed-off-by lines should not be indented
* Commit messages need more details on the reason for the patch (see the changes I've made for examples).
* Commit messages need a 1 line summary at the top (2, 3, and 4 have this, but 1 doesn't)
* There should be a blank line between the 1-lite summary and the remainder of the commit message

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#9 lore raw

First, thanks for accepting 3 of the patches!

19.06.2014 14:14, David Gibson kirjutas:

Quoted 3 lines

Ugh, this is by far the ugliest of these workarounds, and the reason for it in the code is really non-obvious. I'll have to think about this some more.

Good point. Kind of careless of me not to pay attention to that aspect.

What about dropping the field "num_prereqs" altogether? It forces someone looping over the field "prereq" to think about the end condition and comparision with NULL (or nullptr) is an obvious answer. A diff is at the end of the letter.

Quoted 3 lines
Quoted 1 line

* Initialize check->inprogress, too.

Why..?

Stumbled over that one - the value of inprogress was in some cases initialized to "true" when compiled with MSVC.

According to the C99 draft (http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf, pages 137,138), in static storage all fields are initialized to zero, null, 0.

According to MSDN (http://msdn.microsoft.com/en-us/library/81k8cwsz.aspx), initilization is required: "If initializer-list has fewer values than an aggregate type, the remaining members or elements of the aggregate type are initialized to 0. The initial value of an automatic identifier not explicitly initialized is undefined."

Quoted 10 lines

I've applied patches 2, 3, and 4. But for future reference, please remember:

* Patches should be inline, not attachments
* Commit message and signed-off-by lines should not be indented
* Commit messages need more details on the reason for the patch (see the changes I've made for examples).
* Commit messages need a 1 line summary at the top (2, 3, and 4 have this, but 1 doesn't)
* There should be a blank line between the 1-lite summary and the remainder of the commit message

Thanks again.

Is there an automated command to format the commit message per the requirements?

best regards,
Andrei

--------------------------------------------------------

checks.c +8 -10
index 47eda65..fe90dfc 100644
@@ -54,12 +54,11 @@ struct check {
 	bool warn, error;
 	enum checkstatus status;
 	bool inprogress;
-	int num_prereqs;
 	struct check **prereq;
 };
 
 #define CHECK_ENTRY(nm, tfn, nfn, pfn, d, w, e, ...)	       \
-	static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
+	static struct check *nm##_prereqs[] = { __VA_ARGS__, NULL }; \
 	static struct check nm = { \
 		.name = #nm, \
 		.tree_fn = (tfn), \

@@ -69,7 +68,6 @@ struct check {
 		.warn = (w), \
 		.error = (e), \
 		.status = UNCHECKED, \
-		.num_prereqs = ARRAY_SIZE(nm##_prereqs), \
 		.prereq = nm##_prereqs, \
 	};
 #define WARNING(nm, tfn, nfn, pfn, d, ...) \

@@ -153,7 +151,7 @@ static bool run_check(struct check *c, struct node *dt)
 
 	c->inprogress = true;
 
-	for (i = 0; i < c->num_prereqs; i++) {
+	for (i = 0; c->prereq[i] != NULL; i++) {
 		struct check *prq = c->prereq[i];
 		error = error || run_check(prq, dt);
 		if (prq->status != PASSED) {

@@ -192,7 +190,7 @@ static inline void check_always_fail(struct check *c, struct node *dt)
 {
 	FAIL(c, "always_fail check");
 }
-TREE_CHECK(always_fail, NULL);
+TREE_CHECK(always_fail, NULL, NULL);
 
 static void check_is_string(struct check *c, struct node *root,
 			    struct node *node)

@@ -209,9 +207,9 @@ static void check_is_string(struct check *c, struct node *root,
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_STRING(nm, propname) \
-	WARNING(nm, NULL, check_is_string, NULL, (propname))
+	WARNING(nm, NULL, check_is_string, NULL, (propname), NULL)
 #define ERROR_IF_NOT_STRING(nm, propname) \
-	ERROR(nm, NULL, check_is_string, NULL, (propname))
+	ERROR(nm, NULL, check_is_string, NULL, (propname), NULL)
 
 static void check_is_cell(struct check *c, struct node *root,
 			  struct node *node)

@@ -228,9 +226,9 @@ static void check_is_cell(struct check *c, struct node *root,
 		     propname, node->fullpath);
 }
 #define WARNING_IF_NOT_CELL(nm, propname) \
-	WARNING(nm, NULL, check_is_cell, NULL, (propname))
+	WARNING(nm, NULL, check_is_cell, NULL, (propname), NULL)
 #define ERROR_IF_NOT_CELL(nm, propname) \
-	ERROR(nm, NULL, check_is_cell, NULL, (propname))
+	ERROR(nm, NULL, check_is_cell, NULL, (propname), NULL)
 
 /*
  * Structural check functions

@@ -249,7 +247,7 @@ static void check_duplicate_node_names(struct check *c, struct node *dt,
 				FAIL(c, "Duplicate node name %s",
 				     child->fullpath);
 }
-NODE_ERROR(duplicate_node_names, NULL);
+NODE_ERROR(duplicate_node_names, NULL, NULL);
 
 static void check_duplicate_property_names(struct check *c, struct

node *dt,
struct node *node)
@@ -266,7 +264,7 @@ static void check_duplicate_property_names(struct check *c, struct node *dt,
}
}
}
-NODE_ERROR(duplicate_property_names, NULL); +NODE_ERROR(duplicate_property_names, NULL, NULL);

#define LOWERCASE "abcdefghijklmnopqrstuvwxyz"
#define UPPERCASE "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -282,7 +280,7 @@ static void check_node_name_chars(struct check *c, struct node *dt,
FAIL(c, "Bad character '%c' in node %s",
node->name[n], node->fullpath);
}
-NODE_ERROR(node_name_chars, PROPNODECHARS "@"); +NODE_ERROR(node_name_chars, PROPNODECHARS "@", NULL);

static void check_node_name_format(struct check *c, struct node *dt,
struct node *node)
@@ -302,7 +300,7 @@ static void check_property_name_chars(struct check *c, struct node *dt,
FAIL(c, "Bad character '%c' in property name \"%s\", node %s",
prop->name[n], prop->name, node->fullpath);
}
-PROP_ERROR(property_name_chars, PROPNODECHARS); +PROP_ERROR(property_name_chars, PROPNODECHARS, NULL);

#define DESCLABEL_FMT "%s%s%s%s%s"
#define DESCLABEL_ARGS(node,prop,mark) \
@@ -358,7 +356,7 @@ static void check_duplicate_label_prop(struct check *c, struct node *dt,
check_duplicate_label(c, dt, m->ref, node, prop, m);
}
ERROR(duplicate_label, NULL, check_duplicate_label_node,
- check_duplicate_label_prop, NULL);
+ check_duplicate_label_prop, NULL, NULL);

static void check_explicit_phandles(struct check *c, struct node *root,
struct node *node, struct property *prop)
@@ -417,7 +415,7 @@ static void check_explicit_phandles(struct check *c, struct node *root,

node->phandle = phandle;
}
-PROP_ERROR(explicit_phandles, NULL);
+PROP_ERROR(explicit_phandles, NULL, NULL);

static void check_name_properties(struct check *c, struct node *root,
struct node *node)
@@ -649,7 +647,7 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
FAIL(c, "/chosen has obsolete \"interrupt-controller\" "
"property");
}
-TREE_WARNING(obsolete_chosen_interrupt_controller, NULL); +TREE_WARNING(obsolete_chosen_interrupt_controller, NULL, NULL);

static struct check *check_table[] = {
&duplicate_node_names, &duplicate_property_names,
@@ -678,7 +676,7 @@ static void enable_warning_error(struct check *c, bool warn, bool error)

/* Raising level, also raise it for prereqs */
if ((warn && !c->warn) || (error && !c->error))
- for (i = 0; i < c->num_prereqs; i++)
+ for (i = 0; c->prereq[i] != NULL; i++)
enable_warning_error(c->prereq[i], warn, error);

c->warn = c->warn || warn;
@@ -696,7 +694,7 @@ static void disable_warning_error(struct check *c, bool warn, bool error)
struct check *cc = check_table[i];
int j;

- for (j = 0; j < cc->num_prereqs; j++)
+ for (j = 0; cc->prereq[j] != NULL; j++)
if (cc->prereq[j] == c)
disable_warning_error(cc, warn, error);
}

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#11 lore raw

19.06.2014 17:17, Andrei Errapart kirjutas:

Quoted 3 lines
Quoted 3 lines
Quoted 1 line

* Initialize check->inprogress, too.

Why..?

Another option: move the field "inprogress" to the end of the struct. Works, too.

best regards,
Andrei

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#12 lore raw
In reply to a message not in the archive
Quoted 7 lines · Andrei Errapart

First, thanks for accepting 3 of the patches!

19.06.2014 14:14, David Gibson kirjutas:

Quoted 3 lines

Ugh, this is by far the ugliest of these workarounds, and the reason for it in the code is really non-obvious. I'll have to think about this some more.

Quoted 6 lines

Good point. Kind of careless of me not to pay attention to that aspect.

What about dropping the field "num_prereqs" altogether? It forces someone looping over the field "prereq" to think about the end condition and comparision with NULL (or nullptr) is an obvious answer. A diff is at the end of the letter.

Hm, that might work. I'll think on it.

Quoted 10 lines
Quoted 3 lines
Quoted 1 line

* Initialize check->inprogress, too.

Why..?

Stumbled over that one - the value of inprogress was in some cases initialized to "true" when compiled with MSVC.

According to the C99 draft
(http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf, pages 137,138), in static storage all fields are initialized to zero, null, 0.

Exactly..

Quoted 5 lines

According to MSDN (http://msdn.microsoft.com/en-us/library/81k8cwsz.aspx),
initilization is required: "If initializer-list has fewer values than an aggregate type, the remaining members or elements of the aggregate type are initialized to 0. The initial value of an automatic identifier not explicitly initialized is undefined."

Right, an automatic identifier, which is to say a local variable, not a global variable like this,

Quoted 11 lines
Quoted 10 lines

I've applied patches 2, 3, and 4. But for future reference, please remember:

* Patches should be inline, not attachments
* Commit message and signed-off-by lines should not be indented
* Commit messages need more details on the reason for the patch (see the changes I've made for examples).
* Commit messages need a 1 line summary at the top (2, 3, and 4 have this, but 1 doesn't)
* There should be a blank line between the 1-lite summary and the remainder of the commit message

Quoted 4 lines

Thanks again.

Is there an automated command to format the commit message per the requirements?

git format-patch should do it, or git send-email will format and send out the emails for you too.

Quoted 36 lines
checks.c +2 -4
index 47eda65..fe90dfc 100644
@@ -54,12 +54,11 @@ struct check {
 	bool warn, error;
 	enum checkstatus status;
 	bool inprogress;
-	int num_prereqs;
 	struct check **prereq;
 };
 
 #define CHECK_ENTRY(nm, tfn, nfn, pfn, d, w, e, ...)	       \
-	static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \
+	static struct check *nm##_prereqs[] = { __VA_ARGS__, NULL }; \
 	static struct check nm = { \
 		.name = #nm, \
 		.tree_fn = (tfn), \

@@ -69,7 +68,6 @@ struct check {
 		.warn = (w), \
 		.error = (e), \
 		.status = UNCHECKED, \
-		.num_prereqs = ARRAY_SIZE(nm##_prereqs), \
 		.prereq = nm##_prereqs, \
 	};
 #define WARNING(nm, tfn, nfn, pfn, d, ...) \

@@ -153,7 +151,7 @@ static bool run_check(struct check *c, struct node *dt)
 
 	c->inprogress = true;
 
-	for (i = 0; i < c->num_prereqs; i++) {
+	for (i = 0; c->prereq[i] != NULL; i++) {
 		struct check *prq = c->prereq[i];
 		error = error || run_check(prq, dt);
 		if (prq->status != PASSED) {

checks.c static inline void check_always_fail(struct check *c, · line 194

Quoted 6 lines
checks.c +1 -1
@@ -192,7 +190,7 @@ static inline void check_always_fail(struct check *c,
 struct node *dt)
 {
 	FAIL(c, "always_fail check");
 }
-TREE_CHECK(always_fail, NULL);
+TREE_CHECK(always_fail, NULL, NULL);

Why is the extra NULL necessary here (and similar places)?

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

A message that is not in the archive (sent off-list or not yet received): [email protected]

#14 lore raw

Hi,

Quoted 7 lines · David Gibson
Quoted 2 lines · Andrei Errapart

Is there an automated command to format the commit message per the requirements?

git format-patch should do it, or git send-email will format and send out the emails for you too.

Try patman:

http://code.metager.de/source/xref/denx/u-boot/tools/patman/README

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#15 lore raw

PATCH 2/4 Microsoft Visual C patches

In reply to a message not in the archive

commit 8216d97ee62aadac330c9ff85f3775fcd3c760d3

Author: Andrei Errapart <[email protected]>
Date:   Wed Jun 18 18:07:04 2014 +0200

Work around MSVC limitations:
1) No variadic macros in the form "args..."; this is a GCC extension.
2) No empty struct initializers. In any case, there is very little to win: { } vs. { 0 }.

Signed-off-by Andrei Errapart <[email protected]>
dtc.h +3 -3
index 20de073..56212c8 100644
@@ -38,9 +38,9 @@
 #include "util.h"
 
 #ifdef DEBUG
-#define debug(fmt,args...)	printf(fmt, ##args)
+#define debug(...)	printf(__VA_ARGS__)
 #else
-#define debug(fmt,args...)
+#define debug(...)
 #endif
 
 

@@ -88,7 +88,7 @@ struct data {
 };
 
 
-#define empty_data ((struct data){ /* all .members = 0 or NULL */ })
+#define empty_data ((struct data){ 0 /* all .members = 0 or NULL */ })
 
 #define for_each_marker(m) \
 	for (; (m); (m) = (m)->next)
author
#16 lore raw

PATCH 3/4 Microsoft Visual C patches

In reply to a message not in the archive

commit 446c8c45fffdb46a53a6ff9fdfe85311d1209747

Author: Andrei Errapart <[email protected]>
Date:   Wed Jun 18 17:58:22 2014 +0200

Open binary files in binary mode.

Signed-off-by Andrei Errapart <[email protected]>
dtc.c +1 -1
index d36ccdc..e3665b6 100644
@@ -237,7 +237,7 @@ int main(int argc, char *argv[])
 	if (streq(outname, "-")) {
 		outf = stdout;
 	} else {
-		outf = fopen(outname, "w");
+		outf = fopen(outname, "wb");
 		if (! outf)
 			die("Couldn't open output file %s: %s\n",
 			    outname, strerror(errno));
fstree.c +1 -1
index 4d2791c..6d1beec 100644
@@ -52,7 +52,7 @@ static struct node *read_fstree(const char *dirname)
 			struct property *prop;
 			FILE *pfile;
 
-			pfile = fopen(tmpname, "r");
+			pfile = fopen(tmpname, "rb");
 			if (! pfile) {
 				fprintf(stderr,
 					"WARNING: Cannot open %s: %s\n",
srcpos.c +1 -1
index 4549773..f534c22 100644
@@ -77,7 +77,7 @@ static char *try_open(const char *dirname, const char *fname, FILE **fp)
 	else
 		fullname = join_path(dirname, fname);
 
-	*fp = fopen(fullname, "r");
+	*fp = fopen(fullname, "rb");
 	if (!*fp) {
 		free(fullname);
 		fullname = NULL;
author
#17 lore raw

PATCH 4/4 Microsoft Visual C patches

In reply to a message not in the archive

commit 50023c6f8717ee1939ebba885feb9f43fdb7dd33

Author: Andrei Errapart <[email protected]>
Date:   Wed Jun 18 08:21:01 2014 +0200

For Microsoft C compiler, char defaults to signed char.

Signed-off-by Andrei Errapart <[email protected]>
treesource.c +1 -1
index bf7a626..2386b93 100644
@@ -178,7 +178,7 @@ static void write_propval_bytes(FILE *f, struct data val)
 			m = m->next;
 		}
 
-		fprintf(f, "%02hhx", *bp++);
+		fprintf(f, "%02hhx", (unsigned char)(*bp++));
 		if ((const void *)bp >= propend)
 			break;
 		fprintf(f, " ");
author
#18 lore raw
In reply to a message not in the archive

Hi David,

16.06.2014 13:46, David Gibson kirjutas:

Quoted 2 lines

Um.. I don't see what removing the C99 initializers has to do with MSVC not supporting 0 size arrays

Thanks for noticing! Somehow I missed it the last year when MS finally added C99 initializer support to MSVS 2013.

I will clean up the supporting portability shim some later day; it's very small.

best regards,
Andrei Errapart

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html