Name collision in StreamCore.cc

[smarsching]

There is a bug in StreamDevice 2.8.8 when compiling on macOS (10.13.6). The compiler reports the following problem when compiling StreamCore.cc:

../StreamCore.cc:29:19: error: redefinition of 'wait'
    end, in, out, wait, event, exec, connect, disconnect);
                  ^
/usr/include/sys/wait.h:248:7: note: previous definition is here
pid_t   wait(int *) __DARWIN_ALIAS_C(wait);
        ^
../StreamCore.cc:29:19: error: expression is not an integral constant expression
    end, in, out, wait, event, exec, connect, disconnect);
                  ^~~~
../MacroMagic.h:61:83: note: expanded from macro 'ENUM'
  ...type##ToStr(int x) {switch(x){MACRO_FOR_EACH(_CASE_LINE,__VA_ARGS__) def...
                                                             ^~~~~~~~~~~
../MacroMagic.h:55:15: note: expanded from macro 'MACRO_FOR_EACH'
        (x, ##__VA_ARGS__)
              ^~~~~~~~~~~

This problem is caused by the fact that on macOS stdlib.h includes wait.h which declares the wait function. This behavior can be avoided by passing -D_ANSI_SOURCE to the compiler, but doing that causes problems in other places, because it also leads to strncasecmp (and most likely some other functions) to be skipped.

A fix that worked for me is moving the enum definition into a private namespace and then only importing the symbols that do not collide, as implemented by the following patch:

diff -Naur stream-2.8.8.orig/src/StreamCore.cc stream-2.8.8/src/StreamCore.cc
--- stream-2.8.8.orig/src/StreamCore.cc 2018-11-27 13:45:27.000000000 +0100
+++ stream-2.8.8/src/StreamCore.cc  2018-12-11 19:25:13.000000000 +0100
@@ -25,8 +25,21 @@

 #define Z PRINTF_SIZE_T_PREFIX

+namespace streamDeviceEnum {
 ENUM (Commands,
     end, in, out, wait, event, exec, connect, disconnect);
+}
+
+using streamDeviceEnum::Commands;
+using streamDeviceEnum::CommandsToStr;
+using streamDeviceEnum::toStr;
+using streamDeviceEnum::end;
+using streamDeviceEnum::in;
+using streamDeviceEnum::out;
+using streamDeviceEnum::event;
+using streamDeviceEnum::exec;
+using streamDeviceEnum::connect;
+using streamDeviceEnum::disconnect;

 /// debug functions /////////////////////////////////////////////

@@ -50,7 +63,7 @@
                 c = StreamProtocolParser::printString(buffer, c);
                 buffer.append("\";\n");
                 break;
-            case wait:
+            case streamDeviceEnum::wait:
                 timeout = extract<unsigned long>(c);
                 buffer.print("    wait %ld; # ms\n", timeout);
                 break;
@@ -312,7 +325,7 @@
     }
     if (strcmp(command, "wait") == 0)
     {
-        buffer.append(wait);
+        buffer.append(streamDeviceEnum::wait);
         if (!protocol->compileNumber(timeout, args))
         {
             return false;
@@ -553,7 +566,7 @@
         case in:
             //// flags &= ~AcceptEvent;
             return evalIn();
-        case wait:
+        case streamDeviceEnum::wait:
             //// flags &= ~(AcceptInput|AcceptEvent);
             return evalWait();
         case event:

A more elegant solution would be to put the whole code into a namespace, but in this case we would also have to touch all the other files, so that everything belonging to StreamDevice is in the same namespace.

If you would like a PR for the patch included above, or for the alternate solution suggested by me, please let me know.

[dirk-zimoch]

Having to use explicit namespaces everywhere is exactly what I hope to avoid. On the other hand, the commands are ugly C++ anyway. Should be a class instead of an enum with virtual methods instead of if/switch trees.

[smarsching]

As a quick fix, I think it would be sufficient to use an enum class instead of an enum. The elements of an enum class are not visible in the parent namespace and thus there would be no collision. The only downside of this fix is that enum class is a C++ 11 feature and I do not know whether you are willing to drop support for older C++ compilers.

Another fix would be renaming the wait element (e.g. to waitCmd). Actually, this was my first fix, but this has the disadvantage that this also renames the string generated by the preprocessor macros, so debug or error messages would refer to waitCmd instead of wait, which might be misleading.

[dirk-zimoch]

I have put the enum inside the StreamCore class. See commit 26877de. Does this help? (On Linux it compiles, even with sys/wait.h included.) The change is on branch "fix_for_macOS" at the moment. Please check it this solves the problem.

[smarsching]

Yes, this branch fixes the problem for me. Thank you very much for your effort!

[dirk-zimoch]

Merged into master.