wolfSSL / wolfssh

wolfSSH is a small, fast, portable SSH implementation, including support for SCP and SFTP.
https://www.wolfssl.com
378 stars 90 forks source link

Improved "misc.c does not need to be compiled" message #643

Closed gojimmypi closed 9 months ago

gojimmypi commented 10 months ago

As noted in https://github.com/wolfSSL/wolfssh/issues/633, when building with the Espressif ESP-IDF, and the misc.c is not excluded in the CMakeLists.txt, I see this message:

[2/9] Building C object esp-idf/wolfssh/CMakeFiles/__idf_wolfssh.dir/C_/workspace/wolfssh-gojimmypi/src/misc.c.obj
C:/workspace/wolfssh-gojimmypi/src/misc.c:76:10: warning: #warning MISC_WARNING [-Wcpp]
   76 |         #warning MISC_WARNING
      |          ^~~~~~~

In particular, note the MISC_WARNING was previously defined as a descriptive string, but that string is not displayed at compile time.

This PR adds some macro logic to detect if _Pragma is supported, and if so the contents of MISC_WARNING is string-ified and displayed as appropriate.

Although this might have been simplified by simply using the same string in multiple places, there may be value in having the single definition of MISC_WARNING available elsewhere as needed.

Using this change, the warning is manifested like this in my environment:

[2/9] Building C object esp-idf/wolfssh/CMakeFiles/__idf_wolfssh.dir/C_/workspace/wolfssh-gojimmypi/src/misc.c.obj
C:/workspace/wolfssh-gojimmypi/src/misc.c:83:9: note: '#pragma message: misc.c does not need to be compiled when using inline (NO_INLINE not defined))'
   83 |         _Pragma(MISC_TOSTRING(message(MISC_WARNING)))
      |         ^~~~~~~
[3/9] Building C object esp-idf
gojimmypi commented 10 months ago

Any reason to not just use WOLFSSL_IGNORE_FILE_WARN in your user_settings.h? That option covers several other places too.

Yes: my final solution was to exclude the misc.c in the build and not trigger the warning at all. The issue is that when I first saw the "MISC_WARNING" it was not very intuitive. This PR prints the intended message as desired. If there's going to be a warning, it should print the proper text.

ejohnstown commented 10 months ago

I did the change for the MISC_WARNING string. I don't know why I did it that way, but it should probably go back to just being a string on the #warning directive. I suppose we could switch to using _Pragma, but that's going to be the first one. I don't care about duplicating the string there, it isn't compiled code.

gojimmypi commented 10 months ago

I suppose we could switch to using _Pragma, but that's going to be the first one.

I'm ok to go back to the simpler code & duplicate the compile-time text warning. There's otherwise no compelling reason to introduce new things. Just let me know which you prefer.

gojimmypi commented 9 months ago

I've reverted the defined MISC_WARNING macro string to the simpler, duplicated text.