You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/11/03 18:45:18 UTC
[GitHub] [daffodil] tuxji opened a new pull request #673: Raise minimum C level to ISO C11 with GNU extensions
tuxji opened a new pull request #673:
URL: https://github.com/apache/daffodil/pull/673
Fix pedantic warning "ISO C99 doesn't support unnamed structs/unions"
found when compiling nested.dfdl.xsd's NestedUnion example with
current Makefile's CFLAGS. Installed centos7 vagrant box and verified
that both gcc and clang support "-Wall Wextra -Wpedantic -std=gnu11"
all the way back to gcc-4 and clang-3 on CentOS 7. Therefore, will
raise runtime2's minimum C language support level to ISO C11 with GNU
extensions and make all the places where Daffodil calls the C compiler
pass -std=gnu11 to the C compiler to compile code with the same
minimum C level. ISO C11 also allows "for (" loops to define loop
variables in place so we can make that change too.
build.sbt - Change cFlags to use "-Wpendantic -std=gnu11" instead of
"-pedantic -std=gnu99".
Makefile - Change CFLAGS to use "-Wpedantic -std=gnu11" instead of
"-pedantic -std=gnu99". Slightly improve Makefile comments' step by
step instructions as well.
daffodil_getopt.c, xml_reader.c, xml_writer.c, errors.c, infoset.c -
Define loop variables in place within "for (" loops.
CodeGenerator.scala - Pass "-std=gnu11" to C compiler. Now all places
that call the C compiler will compile with the same minimum C level.
NestedUnion/generated_code.[ch] - Replace with re-generated code.
These files are manually updated and their only purpose is to display
how the generated code changes as changes are made to runtime2 over
time.
DAFFODIL-2578
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742306694
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
I changed that struct in a C portability fix commit that went in last May...
<https://github.com/apache/daffodil/commit/8c95b39f5e365ae15657bcdc4686b44689b1fbb9#diff-6df97fc1bdf91db52173653361b5a40e5e075b80312a69912aa23e141dad57c6>
However, these "example" generated code files aren't updated automatically, so I didn't re-generate this example file until now.
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
I changed that struct in a C portability fix commit that went in last May...
<https://github.com/apache/daffodil/commit/8c95b39f5e365ae15657bcdc4686b44689b1fbb9#diff-6df97fc1bdf91db52173653361b5a40e5e075b80312a69912aa23e141dad57c6>
However, these "example" generated code files aren't updated automatically, so I didn't re-generate this example file until now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742306694
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
I changed that struct in a C portability fix commit that went in last May...
<https://github.com/apache/daffodil/commit/8c95b39f5e365ae15657bcdc4686b44689b1fbb9#diff-6df97fc1bdf91db52173653361b5a40e5e075b80312a69912aa23e141dad57c6>
However, these "example" generated code files aren't updated automatically, so I didn't re-generate this example file until now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji merged pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
tuxji merged pull request #673:
URL: https://github.com/apache/daffodil/pull/673
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742285104
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
Hmmm. I'm not seeing a change to a struct such that there is now an "arg" field you have to navigate. Did I just miss it?
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
Hmmm. I'm not seeing a change to a struct such that there is now an "arg" field you have to navigate. Did I just miss it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] stevedlawrence commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742273935
##########
File path: daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/Makefile
##########
@@ -14,56 +14,56 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-# Step 0: You will need to install the Mini-XML library and xmldiff.
-# Here's how to install both packages on Ubuntu 20.04 (first-time
-# setup only):
+# Step 0: Install the Mini-XML library and xmldiff (first time setup
+# only). Here's the command to install both packages on Ubuntu.
# $ sudo apt install libmxml-dev xmldiff
-# Step 1: Copy your test data files here and either rename them to
-# parse.dat and unparse.xml or set PARSE_DAT and UNPARSE_XML.
-
-# $ cp ../ex_nums_parse.dat parse.dat
-# $ cp ../ex_nums_unparse_runtime2.xml unparse.xml
-
-PARSE_DAT = parse.dat
-UNPARSE_XML = unparse.xml
-
-# Step 2: Compile the C source files into an executable program which
-# can run the parse and unparse checks (e.g., .dat <-> .xml).
+# Step 1: Compile the C source files into an executable program which
+# can run the parse and unparse checks (e.g., .dat <-> .xml). Just
+# run make with no arguments unless you want to override CC or CFLAGS.
# $ make
PROGRAM = ./daffodil
HEADERS = libcli/*.h libruntime/*.h
SOURCES = libcli/*.c libruntime/*.c
INCLUDES = -Ilibcli -Ilibruntime
-CFLAGS = -g -Wall -Wextra -pedantic -std=gnu99
+CFLAGS = -g -Wall -Wextra -Wpedantic -std=gnu11
LIBS = -lmxml
$(PROGRAM): $(HEADERS) $(SOURCES)
$(CC) $(CFLAGS) $(INCLUDES) $(SOURCES) $(LIBS) -o $(PROGRAM)
-# Step 3: Run the executable on the test data files and check that the
-# new temp data files match the original test data files.
+# Step 2: Copy your test files here and rename them to parse.dat and
+# unparse.xml or else edit PARSE_DAT and UNPARSE_XML below.
+
+# $ cp ../ex_nums_parse.dat parse.dat
+# $ cp ../ex_nums_unparse_runtime2.xml unparse.xml
+
+PARSE_DAT = parse.dat
+UNPARSE_XML = unparse.xml
Review comment:
Probably not worth changing, but we've found that a naming convention of `foo.ext` for the original file and `foo.ext.xml` for the infoset is sometimes useful. It makes it very clear that that the two files are related and that one is just the "xml" version of the other. In can imagine this could be potentially useful for future Makefile enhancements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742306694
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
I changed that struct in a C portability fix commit that went in last May...
<https://github.com/apache/daffodil/commit/8c95b39f5e365ae15657bcdc4686b44689b1fbb9#diff-6df97fc1bdf91db52173653361b5a40e5e075b80312a69912aa23e141dad57c6>
However, these "example" generated code files aren't updated automatically, so I didn't re-generate this example file until now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] tuxji commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742812372
##########
File path: daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/Makefile
##########
@@ -14,56 +14,56 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-# Step 0: You will need to install the Mini-XML library and xmldiff.
-# Here's how to install both packages on Ubuntu 20.04 (first-time
-# setup only):
+# Step 0: Install the Mini-XML library and xmldiff (first time setup
+# only). Here's the command to install both packages on Ubuntu.
# $ sudo apt install libmxml-dev xmldiff
-# Step 1: Copy your test data files here and either rename them to
-# parse.dat and unparse.xml or set PARSE_DAT and UNPARSE_XML.
-
-# $ cp ../ex_nums_parse.dat parse.dat
-# $ cp ../ex_nums_unparse_runtime2.xml unparse.xml
-
-PARSE_DAT = parse.dat
-UNPARSE_XML = unparse.xml
-
-# Step 2: Compile the C source files into an executable program which
-# can run the parse and unparse checks (e.g., .dat <-> .xml).
+# Step 1: Compile the C source files into an executable program which
+# can run the parse and unparse checks (e.g., .dat <-> .xml). Just
+# run make with no arguments unless you want to override CC or CFLAGS.
# $ make
PROGRAM = ./daffodil
HEADERS = libcli/*.h libruntime/*.h
SOURCES = libcli/*.c libruntime/*.c
INCLUDES = -Ilibcli -Ilibruntime
-CFLAGS = -g -Wall -Wextra -pedantic -std=gnu99
+CFLAGS = -g -Wall -Wextra -Wpedantic -std=gnu11
LIBS = -lmxml
$(PROGRAM): $(HEADERS) $(SOURCES)
$(CC) $(CFLAGS) $(INCLUDES) $(SOURCES) $(LIBS) -o $(PROGRAM)
-# Step 3: Run the executable on the test data files and check that the
-# new temp data files match the original test data files.
+# Step 2: Copy your test files here and rename them to parse.dat and
+# unparse.xml or else edit PARSE_DAT and UNPARSE_XML below.
+
+# $ cp ../ex_nums_parse.dat parse.dat
+# $ cp ../ex_nums_unparse_runtime2.xml unparse.xml
+
+PARSE_DAT = parse.dat
+UNPARSE_XML = unparse.xml
Review comment:
Thanks for the tip. Will remember when creating new data/xml files or changing old data/xml files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742285104
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
Hmmm. I'm not seeing a change to a struct such that there is now an "arg" field you have to navigate. Did I just miss it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil] mbeckerle commented on a change in pull request #673: Raise minimum C level to ISO C11 with GNU extensions
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #673:
URL: https://github.com/apache/daffodil/pull/673#discussion_r742285104
##########
File path: daffodil-runtime2/src/test/resources/org/apache/daffodil/runtime2/examples/NestedUnion/generated_code.c
##########
@@ -365,7 +368,7 @@ data_parseSelf(data *instance, PState *pstate)
break;
default:
// Should never happen because initChoice would return an error first
- error.d64 = (int64_t)instance->_choice;
+ error.arg.d64 = (int64_t)instance->_choice;
Review comment:
Hmmm. I'm not seeing a change to a struct such that there is now an "arg" field you have to navigate. Did I just miss it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org