RIP pre-ANSI C #817
Open
RIP pre-ANSI C #817
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alejandro-colomar
commented
May 23, 2023
•
alejandro-colomar
commented
|
Not sure if you're interested in supporting systems from the early 80's today. If so, please close the PR. Otherwise, I'm happy to simplify the code base a little bit. :) |
alejandro-colomar
force-pushed
the
stdc
branch
3 times, most recently
from
a5fb266
to
d8ea0f4
Compare
tbeu
reviewed
Jun 1, 2023
|
Changes:
|
|
Changes:
|
thesamesam
reviewed
Jun 7, 2023
zconf.h
Outdated
| @@ -265,10 +265,6 @@ | |||
|
|
|||
| /* Type declarations */ | |||
|
|
|||
| #ifndef OF /* function prototypes */ | |||
| # define OF(args) args | |||
There was a problem hiding this comment.
Yes, please. This has caused endless conflicts in other packages.
tbeu
reviewed
Sep 21, 2023
alejandro-colomar
force-pushed
the
stdc
branch
from
62f5ddb
to
154c282
Compare
|
On 2023-09-21 21:13, tbeu wrote:
@tbeu commented on this pull request.
Might be good to rebase and resolve conflicts.
Yup. Rebased.
Also removed NO_ERRNO_H.
$ git range-diff develop..gh/stdc zlib/develop..stdc
1: fff91cf ! 1: 2eb262a RIP pre-ANSI C
@@ contrib/minizip/unzip.c
-#ifdef STDC
-# include <stddef.h>
--# include <string.h>
--# include <stdlib.h>
-#endif
+#include <stddef.h>
-+#include <string.h>
-+#include <stdlib.h>
#ifdef NO_ERRNO_H
extern int errno;
#else
@@ contrib/minizip/zip.c
-#ifdef STDC
-# include <stddef.h>
--# include <string.h>
--# include <stdlib.h>
-#endif
+#include <stddef.h>
-+#include <string.h>
-+#include <stdlib.h>
#ifdef NO_ERRNO_H
extern int errno;
#else
2: af7fb73 = 2: 582315d Fix indentation after previous commit
3: a67481d ! 3: e5bb0f1 NO_snprintf is never defined
@@ gzlib.c: void ZLIB_INTERNAL gz_error(gz_statep state, int err, const char *msg)
#else
## test/minigzip.c ##
-@@ test/minigzip.c: void file_compress(char *file, char *mode) {
+@@ test/minigzip.c: static void file_compress(char *file, char *mode) {
exit(1);
}
@@ test/minigzip.c: void file_compress(char *file, char *mode) {
snprintf(outfile, sizeof(outfile), "%s%s", file, GZ_SUFFIX);
#else
strcpy(outfile, file);
-@@ test/minigzip.c: void file_uncompress(char *file) {
+@@ test/minigzip.c: static void file_uncompress(char *file) {
exit(1);
}
@@ test/minigzip.c: void file_uncompress(char *file) {
snprintf(buf, sizeof(buf), "%s", file);
#else
strcpy(buf, file);
-@@ test/minigzip.c: void file_uncompress(char *file) {
+@@ test/minigzip.c: static void file_uncompress(char *file) {
} else {
outfile = file;
infile = buf;
4: 8e6dd13 = 4: d277547 <stddef.h> is guaranteed by C89
5: db0eaba = 5: a685e2b <stdarg.h> is guaranteed by C89
6: 37f135f = 6: 22f6fba C89 guarantees the return of vsprintf(3)
7: 1714c94 = 7: c883a00 C99 guarantees the return of vsnprintf(3)
8: 249ae7f = 8: b4e099a Remove unused OF() macro
9: 62f5ddb = 9: 3e2d5bf Remove unused Z_ARG() macro
-: ------- > 10: 154c282 <errno.h> is guaranteed by ISO C
…--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
|
|
Would this affect those who compile zlib for the following toolsets though?
|
|
On Wed, Sep 27, 2023 at 02:01:53PM -0700, AraHaan wrote:
Would this affect those who compile zlib for the following toolsets though?
- Visual Studio 2010
- Visual Studio 2012
- Visual Studio 2013
- Visual Studio 2015
- Visual Studio 2017
- Visual Studio 2019
- Visual Studio 2022
I don't know. Do those toolsets support standard C (or at least the
subset we need)?
|
Probably C89 with some Microsoft extensions. |
Then most of the commits can be applied. I assume those extensions will include stdint. Not so sure about vsnprintf(3). |
|
So, I suggest skipping the commit "C99 guarantees the return of vsnprintf(3)". https://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-2010 VS2010 doesn't provide snprintf(3). Other than that, all should be good. AFAIR, VS has been only C89-complying (and probably not even that, but mostly). C99 support was only added recently. This PR is mostly C89-complying, except for the vsnprintf(3) commit, which specifically says C99. Skip that one, and we should be good. Should I rebase? Or will you skip it while applying? |
Oh, I didn't remember. This commit didn't assume that snprintf(3) was present. Only that if it was present it was standards complying. That's still valid with VS2010, which doesn't provide it. We can apply all commits. |
|
@alejandro-colomar, @tbeu, @AraHaan: Z_ARG has been removed: |
All compilers available today have at a minimum C89. The dark times have come to an end. This patch does: - Assume ANSI C. It is now a requirement to build zlib. I wonder if anyone was still testing the old paths by compiling under obsolete compilers for the last decades. - Remove old code that was only triggered if !defined(STDC). - Remove definitions of STDC, since now we assume it's always true. - As a consequence of this patch, vsprintf(3) is now always available, and vsnprintf(3) is available if snprintf(3) is. Signed-off-by: Alejandro Colomar <[email protected]>
I left the indentation in ./configure intact, to make the patch more clear, so that it's visible that it doesn't touch anything that it shouldn't. Fix the indentation now. Signed-off-by: Alejandro Colomar <[email protected]>
The definition of this macro was removed two commits ago. Now, this macro is never defined, so the condition !defined(NO_snprintf) would always be true. However, it might still be the case that snprintf(3) is not available, since it was added in C99, but we're already covered by NO_vsnprintf, since that function was added at the same time. Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
This function wasn't present in C89. It was added in C99, where it
returns the size.
SUSv2 (year 1997) already had vsnprintf(3), and it specified already
a return value. BTW, there was a historic difference that was later
fixed in C99 and POSIX.1-2001, which is mentioned in the HISTORY section
in the manual page in the Linux man-pages, but this detail shouldn't
affect this patch.
snprintf(3):
HISTORY
[...]
snprintf()
vsnprintf()
SUSv2, C99, POSIX.1‐2001.
Concerning the return value of snprintf(), SUSv2 and C99
contradict each other: when snprintf() is called with
size=0 then SUSv2 stipulates an unspecified return value
less than 1, while C99 allows str to be NULL in this
case, and gives the return value (as always) as the num‐
ber of characters that would have been written in case
the output string has been large enough. POSIX.1‐2001
and later align their specification of snprintf() with
C99.
Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar
force-pushed
the
stdc
branch
from
154c282
to
a981f75
Compare
|
Thanks! v5 changes:
|
And ironically, the only file that was calling this function, was also including <limits.h> unconditionally. Signed-off-by: Alejandro Colomar <[email protected]>
gz_intmax() is just a wrapper around INT_MAX. Use the real thing. Signed-off-by: Alejandro Colomar <[email protected]>
|
v6 changes:
|
|
@madler Do you have any thoughts about this patch set? |
|
I am focusing on more critical items. None of this is required for zlib to build correctly. I have made the changes required to avoid warnings due to early adoption of C23. |
|
Hi Mark,
On Thu, Jan 25, 2024 at 09:06:36AM -0800, Mark Adler wrote:
I am focusing on more critical items. None of this is required for
zlib to build correctly. I have made the changes required to avoid
warnings due to early adoption of C23.
Sure, no problem. What I was wondering is if you'd accept the patches
some day, when there are no critical items taking your time, or if you
don't want them at all.
Have a lovely day,
Alex
…--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
|
|
I may eventually apply them, once I look at them more closely. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.