Removed "aacinfo" project also from the solution file + use less obscure <getopt.h> implementation

[lordmulder]

Part 2️⃣

[knik0]

It looks like getopt1.c is not needed.

[lordmulder]

If I exclude getop1.c from the build, then I get linker error:

unresolved external symbol _getopt_long referenced in function _faad_main

I guess you don't "see" that problem with non-MSVC compilers, as those have HAVE_GETOPT_H defined and will use their "system" <getopt.h>, instead of the one from source tree – but MSVC lacks <getopt.h>.

(Note: Opus-Tools and FLAC both have getop.c + getop1.c)

[knik0]

I can't see how it is less obscure, in fact it's more obscure.

[lordmulder]

I can't see how it is less obscure, in fact it's more obscure.

Current implementation:

• including the .h file and the .c into "main.c", instead of making .c a separate compilation unit
• in fact getopt.h is even included twice
• re-defining functions in getopt.c, which already are defined in Std-Lib header files - in an incompatible way that causes compiler warnings (at least on MSVC)

[knik0]

It looks like your "patch" wouldn't even work on all systems lacking getopt which is unacceptable.
Try to think more multiplatform.

[lordmulder]

It looks like your "patch" wouldn't even work on all systems lacking getopt which is unacceptable.
Try to think more multiplatform.

It was back-ported from Xiph.org tools (Opus-Enc, FLAC, etc), so I'm pretty confident that those should be well-tested on all the various platforms where they are relevant.

Note: *NIX platforms (Unix, Linux, etc.) have their own "system" <getopt.h> implementation, so they have HAVE_GETOPT_H defined and therefore don't use the "getopt.h" and "getopt[1].c" from source tree anyway. The "getopt.h" and "getopt[1].c" in source tree are for compatibility to other platforms only.

In any case, #include'ing the <getopt.h> header twice seems like a "copy&paste" mistake to me.

[knik0]

It was back-ported from Xiph.org tools (Opus-Enc, FLAC, etc), so I'm pretty confident that those should be well-tested on all the various platforms.

I'm sure those guys are smart enough to do it the right way but your change doesn't solve any problem yet creates new ones.

Note: *NIX platforms (Unix, Linix, etc.) have their own "system" <getopt.h> implementation, so they have HAVE_GETOPT_H defined and therefore don't use the "getopt.h" and "getopt.c" from source tree anyway. The "getopt.h" and "getopt.c" in source tree are for compatibility to other platforms.

They are likely to have it but I'm not sure it's guaranteed. getopt.h existence is always checked and
HAVE_GETOPT_H is defined accordingly.

[lordmulder]

I'm sure those guys are smart enough to do it the right way but your change doesn't solve any problem yet creates new ones.

Well, it at least solves the problem that current (old) getopt.h re-defines some functions that already have been defined in <stdlib.h>, at least with MSVC. And it does so in an incompatible way, which is why I get warnings. Re-defining functions that are supposed to be defined in standard headers seems dubious to me.

They are likely to have it but I'm not sure it's guaranteed. getopt.h existence is always checked and
HAVE_GETOPT_H is defined accordingly.

Agree. That's why <getopt.h> should be included when HAVE_GETOPT_H is defined, and "getopt.h" should be included otherwise. Currently, though, there is yet another #include <getopt.h> even before the #ifdef HAVE_GETOPT_H ... #else ... #endif guard.