libelektra
libelektra copied to clipboard
Require C99 and update kdbtypes.h for 1.0
I think the 1.0 release would be a good opportunity to require C99 as the minimum C version.
This would also allow as to use stdint.h types in kdbtypes.h, which would simplify the file a lot. In fact, it might make sense to publicly define the kdb_*_t types as aliases for stdint.h types. Then applications could safely use e.g. int32_t instead of kdb_long_t without worrying about compatibility.
Another advantage of requiring C99+ is that we can use stdbool.h everywhere and actually declare functions like keyIsSystem as bool. This means returning false for NULL pointers, but that is actually better than using int and returning -1, since -1 is a true value in C and therefore if (keyIsSystem (NULL)) has unexpected results.
C99 is already the minimum C version when compiling Elektra.
This would also allow as to use stdint.h types in kdbtypes.h, which would simplify the file a lot. In fact, it might make sense to publicly define the kdb_*_t types as aliases for stdint.h types. Then applications could safely use e.g. int32_t instead of kdb_long_t without worrying about compatibility.
Yes, I agree that applications should have the guarantee of this mapping. But don't they already have this guarantee if they compile with C99?
The fallback code without this guarantee should only be relevant for applications compiling with C89. It is clearly separated in kdbtypes.h, so yes the file would get shorter but not simpler?
Another advantage of requiring C99+ is that we can use stdbool.h everywhere and actually declare functions like keyIsSystem as bool. This means returning false for NULL pointers, but that is actually better than using int and returning -1, since -1 is a true value in C and therefore if (keyIsSystem (NULL)) has unexpected results.
We will remove keyIsSystem anyway, so this will not be needed, see #3049
C99 is already the minimum C version when compiling Elektra.
It is not stated in COMPILE.md. But you are write compiling with older versions doesn't work (at least not with gcc).
But don't they already have this guarantee if they compile with C99?
They do, but we don't state it anywhere. So I wouldn't really call it a guarantee.
It is clearly separated in kdbtypes.h, so yes the file would get shorter but not simpler?
The file would get shorter. It would be simpler to read IMO since the C99 stuff is more or less just typedefs and also AFAIK the file could be static and not require CMake's configure_file, if we ignore pre C99 versions.
We will remove keyIsSystem anyway,
Well, keyIsSystem was just an example... There are also other boolean-type functions e.g. keyNeedSync or keyIsBelow.
It is not stated in COMPILE.md. But you are write compiling with older versions doesn't work (at least not with gcc).
Good point. I updated this in 07693378b
They do, but we don't state it anywhere. So I wouldn't really call it a guarantee.
Good point, we are somehow missing a tutorial on how to use types in Elektra. I added it in #280
The file would get shorter. It would be simpler to read IMO since the C99 stuff is more or less just typedefs and also AFAIK the file could be static and not require CMake's configure_file, if we ignore pre C99 versions.
Yes, I agree. But now you added/fixed it, so why remove it? Do you think it might not be working in some cases?
There are also other boolean-type functions e.g. keyNeedSync or keyIsBelow.
It would be possible that we use _Bool in our interface and users of our API decide themselves if they want to include stdbool.h, which might be a big problem for larger code bases. Nevertheless, keyIsBelow is very frequently used, so it would be a lot of work. The benefits in C are very small. So it is a very low-priority issue.
So in total: yes, when we would design a new API directly using C99 types would have been the way to go. As we already have the working ifdefs, I would leave it as-is as there are many other more important issues. Important is to have a tutorial explaining how to use types between different languages. In this tutorial we can presume C99 and directly use C99 types (as in the high-level API).
Things like examples/highlevel/pkgconfig/application.c: const int myint = elektraGetLong (elektra, "myint");we should remove, however. Either C99 fixed-size types or Elektra types.
It would be possible that we use _Bool in our interface and users of our API decide themselves if they want to include stdbool.h, which might be a big problem for larger code bases. Nevertheless, keyIsBelow is very frequently used, so it would be a lot of work. The benefits in C are very small. So it is a very low-priority issue.
The return type isn't that important actually. The main point is that functions like keyIsBelow should only return 0 or 1 but never -1 to make it harder to use it wrongly in ifs. A boolean return type just makes it more obvious that this should be the case.
Ok, so I moved the discussion about the return value to #3123 as it is unrelated to C99 and kdbtypes.h.
@haraldg @beku @machinekoder do you have opinions about if it is okay that we require applications using Elektra to use at least C99? (for the high-level API we already do, for the low-level API currently not)
Yes, using C99 is definitely okay from my POV.
Actually, I'd even prefer it, if it gives us data types, that I recognize on first sight instead of having to look them up somewhere.
Ok, seems like nobody is against it. But please handle it as low-priority issue as better docu should already fix the problem (for C99 users there will not be any difference anyway).
I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart:
ping
I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart:
I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart:
I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart: