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

pluginprocess.c is not C99 #2061

Open
markus2330 opened this Issue Jun 9, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@markus2330
Contributor

markus2330 commented Jun 9, 2018

In the discussions in #2056 we found that pluginprocess.c contains non-C99 code.

  • src/libs/pluginprocess/pluginprocess.c:275:6: warning: implicit declaration of function ‘mkdtemp’ [-Wimplicit-function-declaration
  • src/libs/pluginprocess/pluginprocess.c:275:30: warning: comparison between pointer and integer

EDIT: The second warning seems to be only a aftereffect of the first warning.

@e1528532

This comment has been minimized.

Contributor

e1528532 commented Jun 9, 2018

mkdtemp is probably the best way to create a temporary directory though and i think i had a warning when using mktemp (though i don't remember exactly). It should be defined in stdlib.h present in pluginprocess.c according to https://linux.die.net/man/3/mkdtemp , though its probably newer than C99 i guess. How do we generally deal with such issues?

@markus2330

This comment has been minimized.

Contributor

markus2330 commented Jun 9, 2018

Looking at it more closely, the problem seems to be trivial. Just add a mkdtemp detection in cmake/Modules/FindPluginprocess.cmake (like you already did with mkfifo and fork).

Btw. "FindPluginprocess.cmake" is a very confusing name, and PLUGINPROCESS_FOUND even more so. I expected it searches for a plugin named "process", not that it searches for the dependences of a library called pluginprocess.

Other options are:

  • Move the lib to the plugin processplugin (#1783) or to a binding. Plugin/Bindings can be simply excluded if some dep is not present.
  • We also thought about creating a LIBS CMake variable, that does the same as PLUGINS for libraries.
  • have an alternative implementation for mkdtemp if it is not available.
@ingwinlu

This comment has been minimized.

Contributor

ingwinlu commented Jun 17, 2018

@e1528532 you need the whole man page you linked.

Feature Test Macro Requirements for glibc (see feature_test_macros(7)):
_BSD_SOURCE
|| /* Since glibc 2.10: */
(_POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700)

So yes it is in the header, but not made available unless you fulfill those flags.
In a35813d i implemented those checks for our cframework.

NOTE.: Those are only compatibilities for glibc, though most libc implementations should have 200809 POSIX compatibility they might have other requirements to enable them or enable it automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment