Lmms: need to correct the macros

[xhebox]

As title, obvious the code there is wrong.

--- a/plugins/vst_base/RemoteVstPlugin.cpp  2016-12-10 03:23:56.320741067 +0000
+++ b/plugins/vst_base/RemoteVstPlugin.cpp  2016-12-10 03:23:56.320741067 +0000
@@ -59,13 +59,11 @@
 #define USE_WS_PREFIX
 #include <windows.h>

-#ifdef LMMS_BUILD_WIN32
-#ifdef LMMS_BUILD_WIN64
+#if defined(LMMS_BUILD_WIN32) || defined(LMMS_BUILD_WIN64)
 #include "basename.c"
 #else
 #include <libgen.h>
 #endif
-#endif


 #include <vector>

And if you don't mind, please add this, linux != glibc:

--- a/include/lmms_math.h   2016-12-10 03:20:33.262213993 +0000
+++ b/include/lmms_math.h   2016-12-10 03:20:33.262213993 +0000
@@ -34,7 +34,7 @@
 #include <cmath>
 using namespace std;

-#if defined (LMMS_BUILD_WIN32) || defined (LMMS_BUILD_APPLE) || defined(LMMS_BUILD_HAIKU)  || defined (__FreeBSD__) || defined(__OpenBSD__)
+#if defined (LMMS_BUILD_WIN32) || defined (LMMS_BUILD_APPLE) || defined(LMMS_BUILD_HAIKU)  || defined (__FreeBSD__) || defined(__OpenBSD__) || !defined(__GLIBC__)
 #ifndef isnanf
 #define isnanf(x)  isnan(x)
 #endif

[softrabbit]

If I'm reading this right, you're saying that instead of LMMS_BUILD_WIN32 including libgen.h and LMMS_BUILD_WIN64 including basename.c they both should pull in basename.c and other builds (Linux, OSX etc) should use libgen.h instead of nothing at all?

Does this fix an actual bug?

[xhebox]

Sorry for no explanations.

For me, this fixed the compiling in my build. For you, this is all about correctness and compatibility.

According to the man page, basename() is defined in libgen.h(for posix). When i complied it with musl libc, no such function error broke the build since libgen.h was not included. At least posix systems should include libgen.h instead of nothing in my view. This should make the warning and error messages disappeared when compiling, and passed the strict compiling environment such as my situation.

For windows, there's no basename() in vcrun at all, but mingw. We should just remove basename.c if we believe in mingw. But then, why the developers added this wrapper to lmms? I think it's more safe to use our function instead of depending on mingw.

This won't fix any bug for most people but someone like me. Why not improve the compatibility and get along with more compatible code ?

[xhebox]

Even with glibc, there's warning, if we add a error-as-wrong flag to gcc, we get a broken build then:

s.c: In function 'main':
s.c:5:3: warning: implicit declaration of function 'basename' [-Wimplicit-function-declaration]
   basename("ss");

[tresf]

@xhebox if you create a Pull Request (rather than a bug report), our CI environment will show pass/fail in about 20 minutes for the various environments. Note, if git is too much work, Code Editing and Pull Requests are completely possible via the web. ;)

[xhebox]

@tresf thanks anyway. I did't know that it's possible to send a PR via the web. :D

[tresf]

@xhebox another FYI, when you're ready to issue the PR, if you put Closes #3146 in the description (and if your PR is against the default branch, which it will be), GitHub will close this bug report as soon as we approve the PR.

Note, your commit is sitting out there now. To request it merged, you'll have to click the "Pull Request" button on the patch-1 branch.

https://github.com/LMMS/lmms/compare/master...xhebox:patch-1

[tresf]

@xhebox all green. Merging. 👍

[xhebox]

Glad to see!