Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make C-code in ModelicaMatIO.c C89 compliant #1258

Closed
modelica-trac-importer opened this issue Jan 14, 2017 · 17 comments
Closed

Make C-code in ModelicaMatIO.c C89 compliant #1258

modelica-trac-importer opened this issue Jan 14, 2017 · 17 comments
Labels
enhancement New feature or enhancement L: C-Sources Issue addresses Modelica/Resources/C-Sources
Milestone

Comments

@modelica-trac-importer
Copy link

Reported by jmattsson on 23 Aug 2013 13:45 UTC
ModelicaMatIO.c uses the macros va_copy and strdup which are not a part of the C89 standard. In order to make the code as portable as possible, the code should be changed to be C89 compliant.

There are code already that is supposed to be active when those functions are not available, all that is needed is to activate it for C89 mode.


Migrated-From: https://trac.modelica.org/Modelica/ticket/1258

@modelica-trac-importer modelica-trac-importer added this to the MSL3.2.2 milestone Jan 14, 2017
@modelica-trac-importer modelica-trac-importer added enhancement New feature or enhancement L: C-Sources Issue addresses Modelica/Resources/C-Sources labels Jan 14, 2017
@modelica-trac-importer
Copy link
Author

Comment by jmattsson on 23 Aug 2013 13:52 UTC
ee891d5:
Made code C89 compliant according to patch provided by Thomas Beutlich.

@modelica-trac-importer
Copy link
Author

Comment by jmattsson on 29 Aug 2013 14:12 UTC
d586899:
Reverting ee891d5 since it caused compilation problems on Linux.

@modelica-trac-importer
Copy link
Author

Comment by dietmarw on 6 Dec 2013 15:43 UTC
So I guess this is still open right?

@modelica-trac-importer
Copy link
Author

Comment by anonymous on 11 Dec 2013 11:48 UTC
Yes, still open, but not sure if still relevant.

@modelica-trac-importer
Copy link
Author

Comment by jmattsson on 11 Dec 2013 12:28 UTC
I think it is relevant, since C89 is the only way to make C-code really portable (not enough, but needed).

@modelica-trac-importer
Copy link
Author

Comment by beutlich on 6 Feb 2014 09:17 UTC
Resolved by 4b5e71f.

@modelica-trac-importer
Copy link
Author

Comment by jmattsson on 6 Feb 2014 10:28 UTC
When compiling on Linux, with the command:

gcc -DHAVE_CONFIG_H -I. -Isrc/JMI/src -I../.. -DDUMMY_FUNCTION_USERTAB -DHAVE_ZLIB -Wall -g \
    -Isrc/ThirdParty/MSL/Modelica/Resources/C-Sources -fPIC -MT \
    libModelicaStandardTables_la-ModelicaMatIO.lo -MD -MP -MF \
    .deps/libModelicaStandardTables_la-ModelicaMatIO.Tpo -c \
    src/ThirdParty/MSL/Modelica/Resources/C-Sources/ModelicaMatIO.c  -fPIC -DPIC \
    -o .libs/libModelicaStandardTables_la-ModelicaMatIO.o

We get the errors:

ModelicaMatIO.c: In function 'dopr':
ModelicaMatIO.c:8256:5: error: incompatible types when assigning to type 'va_list' from type 'struct __va_list_tag *'
ModelicaMatIO.c: In function 'mat_vasprintf':
ModelicaMatIO.c:8884:5: error: incompatible types when assigning to type 'va_list' from type 'struct __va_list_tag *'
ModelicaMatIO.c:8892:5: error: incompatible types when assigning to type 'va_list' from type 'struct __va_list_tag *'

Paths in both gcc command and errors have been shortened to make it easier to read.

@modelica-trac-importer
Copy link
Author

Comment by beutlich on 6 Feb 2014 10:49 UTC
Does it work if you delete && __STDC_VERSION__ >= 199901L from line 166 such that we fall back on __va_copy which should be available?

@modelica-trac-importer
Copy link
Author

Comment by jmattsson on 6 Feb 2014 11:17 UTC
Replying to [comment:8 beutlich]:

Does it work if you delete && __STDC_VERSION__ >= 199901L from line 166 such that we fall back on __va_copy which should be available?

Note that was when compiling *without* -std=c89. We'll test your change with and without -std=c89, but I think I remember that the original problem was that __va_copy wasn't available in C89-mode.

@modelica-trac-importer
Copy link
Author

Comment by beutlich on 6 Feb 2014 11:55 UTC
OK, can reproduce with GCC 4.1.2 on CentOS release 5.8 (Final). Changing line 166 as proposed above resolves the issue as __va_copy is available with GCC as of 1997.

@modelica-trac-importer
Copy link
Author

Comment by beutlich on 6 Feb 2014 11:57 UTC
Resolved by 169ec3a.

@modelica-trac-importer
Copy link
Author

Modified by beutlich on 6 Oct 2015 09:23 UTC

@modelica-trac-importer
Copy link
Author

Changelog modified by beutlich on 6 Oct 2015 09:23 UTC
Modelica Standard Tables: Made C-code in ModelicaMatIO.c C89 compliant

@modelica-trac-importer
Copy link
Author

Comment by beutlich on 15 Jan 2016 08:35 UTC
After testing for #1880 I noticed that C89 compliance is broken again:

ModelicaMatIO.c: In function "Mat_VarDelete":
ModelicaMatIO.c:10490: warning: implicit declaration of function "mktemp"

Defining _GNU_SOURCE to 1 helped to get rid of this warning again. Is this OK for you? If not, I might switch to tmpnam (instead of mktemp) again.

@modelica-trac-importer
Copy link
Author

Comment by jmattsson on 15 Jan 2016 10:38 UTC
Replying to [comment:13 beutlich]:

Is this OK for you? If not, I might switch to tmpnam (instead of mktemp) again.

OK for us.

@modelica-trac-importer
Copy link
Author

Comment by sjoelund.se on 15 Jan 2016 10:45 UTC
Then we would get warnings for all simulations linked against ModelicaMatIO:

/tmp/a-c745af.o: In function `main':
a.c:(.text+0x12): warning: the use of `tmpnam' is dangerous, better use `mkstemp'

The reason is that there is a race condition after you get the temporary name and open the file (tmpnam might return the same name again).

@modelica-trac-importer
Copy link
Author

Comment by beutlich on 15 Jan 2016 13:25 UTC
Resolved by d24b63c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement L: C-Sources Issue addresses Modelica/Resources/C-Sources
Projects
None yet
Development

No branches or pull requests

1 participant