Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

6.2.0 is less embedded friendly in comparison to 5.3.0 #1654

Closed
kwesolowski opened this issue Apr 29, 2020 · 3 comments
Closed

6.2.0 is less embedded friendly in comparison to 5.3.0 #1654

kwesolowski opened this issue Apr 29, 2020 · 3 comments

Comments

@kwesolowski
Copy link
Contributor

Hi,

Playing with IAR and GHS compilers I see some places (mostly os.h/os.cpp) where for example:

EINTR retry is done for "not windows", FILE * operations are included etc. Most of the functionality is perfectly "in RAM", but those few pieces leak os related items. Would you like to reduce such "heave OS" dependencies? What would be preferred Pull Request?

@kwesolowski
Copy link
Contributor Author

kwesolowski commented Apr 29, 2020

Example patch (definitions would need to move outside lib). Or maybe just better mechanism to exclude os dependant features?

+#define FMT_RETRY(result, expression) result = (expression)
+#define FMT_RETRY_VAL(result, expression, error_result) result = (expression)
+
+#ifndef FMT_RETRY
 // Retries the expression while it evaluates to error_result and errno
 // equals to EINTR.
 #ifndef _WIN32
 #  define FMT_RETRY_VAL(result, expression, error_result) \
     do {                                                  \
@@ -69,10 +73,11 @@
 #else
 #  define FMT_RETRY_VAL(result, expression, error_result) result = (expression)
 #endif
 
 #define FMT_RETRY(result, expression) FMT_RETRY_VAL(result, expression, -1)
+#endif

@vitaut
Copy link
Contributor

vitaut commented Apr 29, 2020

os.h/os.cc are non-essential and can be excluded from the build. You only need format.cc and headers. I am open to a PR to add a CMake flag (e.g. FMT_OS) to control whether to include OS-specific APIs.

@kwesolowski
Copy link
Contributor Author

Will make a PR.

kwesolowski added a commit to rainlabs-eu/fmt that referenced this issue Apr 30, 2020
kwesolowski added a commit to rainlabs-eu/fmt that referenced this issue Apr 30, 2020
kwesolowski added a commit to rainlabs-eu/fmt that referenced this issue Apr 30, 2020
- Allows to avoid part of fmtlib#1654
- Not possible to use with MASTER_PROJECT as testing uses it a lot
vitaut pushed a commit that referenced this issue May 1, 2020
- Allows to avoid part of #1654
- Not possible to use with MASTER_PROJECT as testing uses it a lot
@vitaut vitaut closed this as completed May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants