Skip to content

Raise minimum C level to ISO C11 with GNU extensions#673

Merged
tuxji merged 1 commit intoapache:mainfrom
tuxji:ji/misc-runtime2-fixes
Nov 4, 2021
Merged

Raise minimum C level to ISO C11 with GNU extensions#673
tuxji merged 1 commit intoapache:mainfrom
tuxji:ji/misc-runtime2-fixes

Conversation

@tuxji
Copy link
Contributor

@tuxji tuxji commented Nov 3, 2021

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

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
Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. Looks fine.

This C language/library stuff has a big Y2K bug: 11 > 99 because 11 => 2011, and 99 => 1999. Gaaaaaak. :-)

default:
// Should never happen because initChoice would return an error first
error.d64 = (int64_t)instance->_choice;
error.arg.d64 = (int64_t)instance->_choice;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed that struct in a C portability fix commit that went in last May...

8c95b39#diff-6df97fc1bdf91db52173653361b5a40e5e075b80312a69912aa23e141dad57c6

However, these "example" generated code files aren't updated automatically, so I didn't re-generate this example file until now.

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check I see that failed is due to a race-condition in one of the tests I wrote where I assumed 100msec would be a long enough wait time.... but it seems sometimes it isn't if the test machine is perhaps heavily loaded.

Copy link
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

# $ cp ../ex_nums_unparse_runtime2.xml unparse.xml

PARSE_DAT = parse.dat
UNPARSE_XML = unparse.xml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip. Will remember when creating new data/xml files or changing old data/xml files.

@tuxji tuxji merged commit 4fd08c6 into apache:main Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants