Target operating system defines are check for presence to prevent war…

[ppisa]

…nings

The conditionalized code define checks for specific target changed to pattern with initial check for define presence

#if defined(ELKS) && ELKS

This resolves -Wundef warnings.

[ppisa]

This is an attempt to resolve lot of warning when inactive target macros are not initialized to zero.

This solves lot of these warnings for NuttX porting

apache/nuttx-apps#3624

There has been found even some more forgotten #ifdefs which could lead to incorrect conditions evaluation in past.

--- a/src/contrib/doom/doomtype.h
+++ b/src/contrib/doom/doomtype.h
@@ -45,7 +45,7 @@ typedef unsigned char byte;


 // Predefined with some OS.
-#ifdef LINUX
+#if defined(LINUX) && LINUX
 #include <values.h>
 #else
 #ifndef MAXCHAR

--- a/src/demos/nanox/nxtetris.h
+++ b/src/demos/nanox/nxtetris.h
@@ -38,7 +38,7 @@
  * array of shape descriptions (you can add your own new shapes quite easily).
  */

-#ifndef __ECOS
+#if !defined(__ECOS) || !__ECOS
 //#define USE_HISCORE_FILE
 #define HISCORE_FILE "/usr/games/nanotetris.hiscore"
 #endif
@@ -58,7 +58,7 @@
 extern int WELL_HEIGHT, WELL_VISIBLE_HEIGHT;
 #define WELL_NOTVISIBLE (WELL_HEIGHT - WELL_VISIBLE_HEIGHT)
 #define LEVEL_DIVISOR 500
-#ifdef __ECOS
+#if defined(__ECOS) && __ECOS
 #define DROP_BLOCK_DELAY 10
 #else
 #define DROP_BLOCK_DELAY 25

But generally it is lot of changes and makes the code a little less readable. So it is a question if this is a right direction. @ghaerr, is this change acceptable for you? Do you refer to attempt another direction? Unset defines can be zeroed for example in mwconfig.h but there would be lot of situations where mwconfig.h is not included.

If we consider that some more systems can be added in future then each such addition would require to add zero define of new target related macro to all other configurations without this change. Which is not ideal.

[ghaerr]

Hello @ppisa,

I'm sorry, but I am not in favor of this modification, touching 58 files in the main repo, just for the sake of getting
-Wundef to display more nicely on the external NuttX repo.

This approach leads to even more messiness like the suggested fix at the top of nxterm.h:

#if defined(LINUX) && LINUX
#define USE_NGTERM 1
#elif defined(MACOSX) && MACOSX
#define USE_NGTERM 1
#else
#define USE_NGTERM 0
#endif

#if USE_NGTERM

This is not a style I'd like to require following in this repo.

Also, the source base would have to contend with always rewriting something like:

#ifndef __ECOS

with the harder-to-comprehend and visually obtuse:

#if !defined(__ECOS) || !__ECOS

Using just a system of #ifdef or #ifndef, along with its more complex #if defined(SYM) && !defined(OTHER) would be superior, but this breaks all the external Makefiles using -DSYM=1 and header files defining symbols to be 1 or 0 to turn features on and off. So this doesn't work well either. And that's why this approach has been used in Microwindows.

I can't remember if #if defined(SYM) rather than #ifdef SYM worked in all versions of the 20 different versions off UNIX C compilers available in the days the majority of Microwindows was coded, but that may be how it started.

Thank you!

[ghaerr]

There has been found even some more forgotten #ifdefs which could lead to incorrect conditions evaluation in past.

Despite my earlier comments, I would agree that there remains considerable potential for cleaning up Microwindows sources, most notably some of the applications. But then these applications are mostly for demo. I have not contributed much to the applications lately, as my time is consumed with newer projects. I could be for cleaning up the applications, but this is also a potential pit of complexity as well.

Unset defines can be zeroed for example in mwconfig.h but there would be lot of situations where mwconfig.h is not included.

Do you have a list of these cases in applications, or is this in the core engine files? In general, the Microwindows/Nano-X engine configuration file should not be exported to applications. I can comment further once seeing the list.

I can agree with your point about potential problems with mwconfig.h not included, but sorting header files can quickly get to be a complicated project, and I remain very concerned about maintaining (current) correctness while trying to fix a problem that involves changing or adding lots of header files in lots of places. In the ELKS project which I maintain, I have been steadily working for 5+ years trying to sort out and slim down the dependency requirements of the system and library header files, and have made great progress, but have also unwittingly introduced build failures and subtle bugs during the process. The problem with doing this in this repo is that most of the targets can't be rebuilt or tested easily.

All that said, the entire recursive Makefile system contributed long ago here has outgrown its usefulness, and likely needs a complete rewrite - which is why I suggested moving to a much simpler-to-maintain Makefile.nuttx etc approach.
But I worry that there are lots of very old users of Microwindows/Nano-X that may use the older system and making a major change at this point will cause their builds to break when doing a git pull.

All-in-all, this whole subject is quite complicated and has lots of pitfalls, but could still be improved.

f we consider that some more systems can be added in future then each such addition would require to add zero define of new target related macro to all other configurations without this change. Which is not ideal.

I agree, but I also do not think the answer should be adding lots of zero defines in config files (other than your NuttX config file, of course). Does that mean we just have a few cases to consider regarding missing mwconfig.h, or is this rampant throughout the repo? I don't want to include unnecessary includes just to stop warnings, as this may introduce header file dependencies which aren't really required. But I don't know enough to fully understand how much is amiss in this particular case.

What is the big issue here? Is it just that -Wundef is incompatible with the Microwindows repo and spits out tons of warnings, or are there other real correctness issues that need to be addressed, or both?

[ghaerr]

Since this problem is pretty much entirely tied to the NuttX system builder's requirement of using -Wundef, perhaps consider that instead of rewriting header files, include hierarchies, and generally adding lots of changes to this repo, the entire problem can actually be pretty easily solved entirely on the NuttX end: just add a (large) string of -DRTEMS=0 -DLINUX=0 etc to your compiler command line via a simple macro in Make.defs, which gets expanded to the (large) list of defines required to satisfy the -Wundef (or vice versa with Make.defs vs a Makefile).

TBO, the more I think about it, the more I like this approach. With a simple change to your own build system, you can, without changing the upstream repo to meet your specialized build, likely cleanup most all the warnings. It would have the potential advantage that any subsequent updates to this repo might then emit a warning on your build, which would be a good trigger to determine whether it mattered, etc.

For the truly egregious cases which indicate a real error on Microwindows' part, those could be corrected on this side. These changes if any would be small, not like including mwconfig.h everywhere but only fixing true errors which aren't easily fixed with an additional -D in your build.

I saw the discussion on where, why and why not include -D compiler options on the command line, how does this idea fit with the results of that discussion?

[ppisa]

OK, I understand that this is too much changes to suppress warnings and code is less readable. On the other hand, it is possible that -Wudef would be widely adopted to became default one day. But yes, for projects where Microwindows are main system/external component it is no problem to keep CFLAGS to support it. In the case of NuttX, there are lot of other core and optional components and I understand to maintainers that they do not like that one component makes CFLAGS line two or may be three times longer.

I will look for the leftover cases in the sources where #ifdef LINUX and similar are used to resolve these cases where define 0 leads to incorrect results.

[ghaerr]

it is possible that -Wudef would be widely adopted to became default one day.

What exactly does -Wundef do? Warning when a symbol is undefined when also its value is used when automatically defined to be 0 in an #if SYM expression? That would seem to prohibit the use of #if directives and force the use of #ifdef instead - effectively deprecating a C feature. Is this part of any standard?

I will look for the leftover cases in the sources where #ifdef LINUX and similar are used to resolve these cases where define 0 leads to incorrect results.

That's fine. The particular case of #ifdef LINUX is in contrib/doom and corresponds to external source code included in this repo, but not maintained by this repo; thus these kinds of things should not be changed. Only core directories, headers and source files both included in the NuttX build and where there are specifically cases of using the symbols improperly (e.g. uses of both #if SYM and also #ifdef SYM) should be corrected, right?