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

RIP pre-ANSI C #817

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Conversation

alejandro-colomar
Copy link

@alejandro-colomar alejandro-colomar commented May 23, 2023

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.

Revisions:

v6b
  • Rebase
$ git range-diff 5bb6819^..gh/stdc master..stdc 
 1:  5bb6819 =  1:  857148c RIP pre-ANSI C
 2:  ab7a569 =  2:  bbbb8bb Fix indentation after previous commit
 3:  44eda1a =  3:  6b34834 NO_snprintf is never defined
 4:  fa45ad7 =  4:  4a4a4da <stddef.h> is guaranteed by C89
 5:  9990f65 =  5:  c84e041 <stdarg.h> is guaranteed by C89
 6:  86fd62f =  6:  573b0ba C89 guarantees the return of vsprintf(3)
 7:  a9310aa =  7:  765a775 C99 guarantees the return of vsnprintf(3)
 8:  18a5141 =  8:  9776cea Remove unused OF() macro
 9:  a981f75 =  9:  f7f0cca <errno.h> is guaranteed by ISO C
10:  90b5f2d = 10:  8ee65ef <limits.h> and INT_MAX are guaranteed by C89
11:  3d86bb3 ! 11:  fd93fe7 Use INT_MAX directly
    @@ zconf.h.in
     
      ## zlib.map ##
     @@ zlib.map: ZLIB_1.2.0 {
    -     zcfree;
    -     z_errmsg;
    -     gz_error;
    --    gz_intmax;
    -     _*;
    - };
    - 
    +     zcfree;
    +     z_errmsg;
    +     gz_error;
    +-    gz_intmax;
    +     _*;
    + };
    + 
v6c
  • Rebase
$ git range-diff master..gh/stdc develop..stdc 
 1:  857148c !  1:  7761efe RIP pre-ANSI C
    @@ gzguts.h
     +#include <stdlib.h>
     +#include <limits.h>
      
    - #ifndef _POSIX_SOURCE
    - #  define _POSIX_SOURCE
    + #ifndef _POSIX_C_SOURCE
    + #  define _POSIX_C_SOURCE 200112L
     @@
         define "local" for the non-static meaning of "static", for readability
         (compile with -Dlocal if your debugger can't find static symbols) */
    @@ test/example.c
     +#include <string.h>
     +#include <stdlib.h>
      
    - #if defined(VMS) || defined(RISCOS)
    + #if defined(VMS)
      #  define TESTFILE "foo-gz"
     
      ## test/minigzip.c ##
    @@ zconf.h
     -#  endif
     -#endif
      
    - #if defined(ZLIB_CONST) && !defined(z_const)
    - #  define z_const const
    + #ifndef z_const
    + #  ifdef ZLIB_CONST
     @@
      #  define z_longlong long long
      #  if defined(NO_SIZE_T)
    @@ zconf.h.cmakein
     -#  endif
     -#endif
      
    - #if defined(ZLIB_CONST) && !defined(z_const)
    - #  define z_const const
    + #ifndef z_const
    + #  ifdef ZLIB_CONST
     @@
      #  define z_longlong long long
      #  if defined(NO_SIZE_T)
    @@ zconf.h.in
     -#  endif
     -#endif
      
    - #if defined(ZLIB_CONST) && !defined(z_const)
    - #  define z_const const
    + #ifndef z_const
    + #  ifdef ZLIB_CONST
     @@
      #  define z_longlong long long
      #  if defined(NO_SIZE_T)
 2:  bbbb8bb =  2:  20d9e6c Fix indentation after previous commit
 3:  6b34834 !  3:  b46444c NO_snprintf is never defined
    @@ Commit message
     
      ## gzlib.c ##
     @@ gzlib.c: local gzFile gz_open(const void *path, int fd, const char *mode) {
    -             *(state->path) = 0;
          else
      #endif
    +     {
     -#if !defined(NO_snprintf) && !defined(NO_vsnprintf)
     +#if !defined(NO_vsnprintf)
              (void)snprintf(state->path, len + 1, "%s", (const char *)path);
    @@ gzlib.c: void ZLIB_INTERNAL gz_error(gz_statep state, int err, const char *msg)
          (void)snprintf(state->msg, strlen(state->path) + strlen(msg) + 3,
                         "%s%s%s", state->path, ": ", msg);
      #else
    -
    - ## test/minigzip.c ##
    -@@ test/minigzip.c: static void file_compress(char *file, char *mode) {
    -         exit(1);
    -     }
    - 
    --#if !defined(NO_snprintf) && !defined(NO_vsnprintf)
    -+#if !defined(NO_vsnprintf)
    -     snprintf(outfile, sizeof(outfile), "%s%s", file, GZ_SUFFIX);
    - #else
    -     strcpy(outfile, file);
    -@@ test/minigzip.c: static void file_uncompress(char *file) {
    -         exit(1);
    -     }
    - 
    --#if !defined(NO_snprintf) && !defined(NO_vsnprintf)
    -+#if !defined(NO_vsnprintf)
    -     snprintf(buf, sizeof(buf), "%s", file);
    - #else
    -     strcpy(buf, file);
    -@@ test/minigzip.c: static void file_uncompress(char *file) {
    -     } else {
    -         outfile = file;
    -         infile = buf;
    --#if !defined(NO_snprintf) && !defined(NO_vsnprintf)
    -+#if !defined(NO_vsnprintf)
    -         snprintf(buf + len, sizeof(buf) - len, "%s", GZ_SUFFIX);
    - #else
    -         strcat(infile, GZ_SUFFIX);
    -@@ test/minigzip.c: int main(int argc, char *argv[]) {
    -     gzFile file;
    -     char *bname, outmode[20];
    - 
    --#if !defined(NO_snprintf) && !defined(NO_vsnprintf)
    -+#if !defined(NO_vsnprintf)
    -     snprintf(outmode, sizeof(outmode), "%s", "wb6 ");
    - #else
    -     strcpy(outmode, "wb6 ");
 4:  4a4a4da =  4:  b77dda3 <stddef.h> is guaranteed by C89
 5:  c84e041 =  5:  2874322 <stdarg.h> is guaranteed by C89
 6:  573b0ba =  6:  6e4511d C89 guarantees the return of vsprintf(3)
 7:  765a775 =  7:  b66b5ac C99 guarantees the return of vsnprintf(3)
 8:  9776cea =  8:  41f0312 Remove unused OF() macro
 9:  f7f0cca =  9:  e6a867f <errno.h> is guaranteed by ISO C
10:  8ee65ef = 10:  5f65830 <limits.h> and INT_MAX are guaranteed by C89
11:  fd93fe7 = 11:  2f076dd Use INT_MAX directly
v7
  • Drop patch 3. Some other changes have been applied a few months ago that make this patch obsolete.
  • Remove some more cases in patch 1.
$ git range-diff develop gh/stdc stdc 
 1:  7761efe !  1:  f8f44db RIP pre-ANSI C
    @@ contrib/minizip/zip.c
          extern int errno;
      #else
     
    + ## crc32.c ##
    +@@ crc32.c: local z_crc_t FAR crc_table[256];
    + typedef struct once_s once_t;
    + 
    + /* Check for the availability of atomics. */
    +-#if defined(__STDC__) && __STDC_VERSION__ >= 201112L && \
    +-    !defined(__STDC_NO_ATOMICS__)
    ++#if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_ATOMICS__)
    + 
    + #include <stdatomic.h>
    + 
    +
      ## deflate.h ##
     @@ deflate.h: void ZLIB_INTERNAL _tr_stored_block(deflate_state *s, charf *buf,
      #ifndef ZLIB_DEBUG
    @@ zutil.h: typedef unsigned short ush;
      #  include <limits.h>
      #  if (ULONG_MAX == 0xffffffffffffffff)
      #    define Z_U8 unsigned long
    +@@ zutil.h: extern z_const char * const z_errmsg[10]; /* indexed by 2-zlib_error */
    + #  define OS_CODE  0x00
    + #  ifndef Z_SOLO
    + #    if defined(__TURBOC__) || defined(__BORLANDC__)
    +-#      if (__STDC__ == 1) && (defined(__LARGE__) || defined(__COMPACT__))
    ++#      if defined(__LARGE__) || defined(__COMPACT__)
    +          /* Allow compilation with ANSI keywords only enabled */
    +          void _Cdecl farfree( void *block );
    +          void *_Cdecl farmalloc( unsigned long nbytes );
     @@ zutil.h: extern z_const char * const z_errmsg[10]; /* indexed by 2-zlib_error */
        */
      #  define NO_MEMCPY
 2:  20d9e6c =  2:  752859e Fix indentation after previous commit
 3:  b46444c <  -:  ------- NO_snprintf is never defined
 4:  b77dda3 =  3:  13a195d <stddef.h> is guaranteed by C89
 5:  2874322 =  4:  c4242d7 <stdarg.h> is guaranteed by C89
 6:  6e4511d =  5:  b813ec1 C89 guarantees the return of vsprintf(3)
 7:  b66b5ac =  6:  bce585e C99 guarantees the return of vsnprintf(3)
 8:  41f0312 =  7:  ceafc70 Remove unused OF() macro
 9:  e6a867f =  8:  7efa964 <errno.h> is guaranteed by ISO C
10:  5f65830 =  9:  89b6a3c <limits.h> and INT_MAX are guaranteed by C89
11:  2f076dd = 10:  e68185f Use INT_MAX directly

@alejandro-colomar alejandro-colomar marked this pull request as ready for review May 23, 2023 23:05
@alejandro-colomar
Copy link
Author

alejandro-colomar commented May 23, 2023

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 alejandro-colomar force-pushed the stdc branch 3 times, most recently from a5fb266 to d8ea0f4 Compare May 30, 2023 13:09
CMakeLists.txt Show resolved Hide resolved
@alejandro-colomar
Copy link
Author

Changes:

  • Drop <stdint.h> changes. It is C99 stuff. [@tbeu]
$ git range-diff develop..gh/stdc develop..stdc 
 1:  3702222 =  1:  3702222 RIP pre-ANSI C
 2:  f2a4676 =  2:  f2a4676 Fix indentation after previous commit
 3:  2907307 =  3:  2907307 <stddef.h> is guaranteed by C89
 4:  de72975 <  -:  ------- <stdint.h> is guaranteed by C89
 5:  78b5dc9 =  4:  ed203a2 <stdarg.h> is guaranteed by C89
 6:  db569e5 =  5:  366b679 C89 guarantees the return of vsprintf(3)
 7:  25f2018 =  6:  01bed35 C99 guarantees the return of vsnprintf(3)
 8:  4f57065 =  7:  3ab80c3 Remove unused OF() macro
 9:  d8ea0f4 =  8:  d5ae4e8 Remove unused Z_ARG() macro

@alejandro-colomar
Copy link
Author

alejandro-colomar commented Jun 6, 2023

Changes:

  • Remove spurious configure message for test that I had removed.
  • Update comment.
  • Remove always-true conditional, since we removed the corresponding
    macro definition.

@thesamesam
Copy link

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. :)

See also e9d5486 (#633).

zconf.h Outdated
@@ -265,10 +265,6 @@

/* Type declarations */

#ifndef OF /* function prototypes */
# define OF(args) args

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please. This has caused endless conflicts in other packages.

Copy link
Contributor

@tbeu tbeu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to rebase and resolve conflicts.

@alejandro-colomar
Copy link
Author

alejandro-colomar commented Sep 22, 2023 via email

@AraHaan
Copy link
Contributor

AraHaan commented Sep 27, 2023

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

@alejandro-colomar
Copy link
Author

alejandro-colomar commented Sep 27, 2023 via email

@AraHaan
Copy link
Contributor

AraHaan commented Sep 28, 2023

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.

@alejandro-colomar
Copy link
Author

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).

@alejandro-colomar
Copy link
Author

@AraHaan

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?

@alejandro-colomar
Copy link
Author

@AraHaan

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.

@Neustradamus
Copy link

@alejandro-colomar, @tbeu, @AraHaan: Z_ARG has been removed:

@alejandro-colomar
Copy link
Author

Thanks!

v5 changes:

  • Rebase to madler/develop
$ git range-diff develop..alx/stdc madler/develop..stdc 
 1:  2eb262a !  1:  5bb6819 RIP pre-ANSI C
    @@ zconf.h
     +#  define OF(args)  args
      #endif
      
    - #ifndef Z_ARG /* function prototypes for stdarg */
    --#  if defined(STDC) || defined(Z_HAVE_STDARG_H)
    --#    define Z_ARG(args)  args
    --#  else
    --#    define Z_ARG(args)  ()
    --#  endif
    -+#  define Z_ARG(args)  args
    - #endif
    - 
      /* The following definitions for FAR are needed only for MSDOS mixed
     @@ zconf.h: typedef int   FAR intf;
      typedef uInt  FAR uIntf;
    @@ zconf.h.cmakein
     +#  define OF(args)  args
      #endif
      
    - #ifndef Z_ARG /* function prototypes for stdarg */
    --#  if defined(STDC) || defined(Z_HAVE_STDARG_H)
    --#    define Z_ARG(args)  args
    --#  else
    --#    define Z_ARG(args)  ()
    --#  endif
    -+#  define Z_ARG(args)  args
    - #endif
    - 
      /* The following definitions for FAR are needed only for MSDOS mixed
     @@ zconf.h.cmakein: typedef int   FAR intf;
      typedef uInt  FAR uIntf;
    @@ zconf.h.in
     +#  define OF(args)  args
      #endif
      
    - #ifndef Z_ARG /* function prototypes for stdarg */
    --#  if defined(STDC) || defined(Z_HAVE_STDARG_H)
    --#    define Z_ARG(args)  args
    --#  else
    --#    define Z_ARG(args)  ()
    --#  endif
    -+#  define Z_ARG(args)  args
    - #endif
    - 
      /* The following definitions for FAR are needed only for MSDOS mixed
     @@ zconf.h.in: typedef int   FAR intf;
      typedef uInt  FAR uIntf;
 2:  582315d =  2:  ab7a569 Fix indentation after previous commit
 3:  e5bb0f1 =  3:  44eda1a NO_snprintf is never defined
 4:  d277547 =  4:  fa45ad7 <stddef.h> is guaranteed by C89
 5:  a685e2b =  5:  9990f65 <stdarg.h> is guaranteed by C89
 6:  22f6fba =  6:  86fd62f C89 guarantees the return of vsprintf(3)
 7:  c883a00 =  7:  a9310aa C99 guarantees the return of vsnprintf(3)
 8:  b4e099a <  -:  ------- Remove unused OF() macro
 9:  3e2d5bf !  8:  18a5141 Remove unused Z_ARG() macro
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    Remove unused Z_ARG() macro
    +    Remove unused OF() macro
     
         Signed-off-by: Alejandro Colomar <alx@nginx.com>
     
    @@ zconf.h
      
                              /* Type declarations */
      
    --#ifndef Z_ARG /* function prototypes for stdarg */
    --#  define Z_ARG(args)  args
    +-#ifndef OF /* function prototypes */
    +-#  define OF(args)  args
     -#endif
     -
      /* The following definitions for FAR are needed only for MSDOS mixed
    @@ zconf.h.cmakein
      
                              /* Type declarations */
      
    --#ifndef Z_ARG /* function prototypes for stdarg */
    --#  define Z_ARG(args)  args
    +-#ifndef OF /* function prototypes */
    +-#  define OF(args)  args
     -#endif
     -
      /* The following definitions for FAR are needed only for MSDOS mixed
    @@ zconf.h.in
      
                              /* Type declarations */
      
    --#ifndef Z_ARG /* function prototypes for stdarg */
    --#  define Z_ARG(args)  args
    +-#ifndef OF /* function prototypes */
    +-#  define OF(args)  args
     -#endif
     -
      /* The following definitions for FAR are needed only for MSDOS mixed
10:  154c282 =  9:  a981f75 <errno.h> is guaranteed by ISO C

@alejandro-colomar
Copy link
Author

v6 changes:

  • Use INT_MAX directly
$ git range-diff madler/develop alx/stdc stdc 
 1:  5bb6819 =  1:  5bb6819 RIP pre-ANSI C
 2:  ab7a569 =  2:  ab7a569 Fix indentation after previous commit
 3:  44eda1a =  3:  44eda1a NO_snprintf is never defined
 4:  fa45ad7 =  4:  fa45ad7 <stddef.h> is guaranteed by C89
 5:  9990f65 =  5:  9990f65 <stdarg.h> is guaranteed by C89
 6:  86fd62f =  6:  86fd62f C89 guarantees the return of vsprintf(3)
 7:  a9310aa =  7:  a9310aa C99 guarantees the return of vsnprintf(3)
 8:  18a5141 =  8:  18a5141 Remove unused OF() macro
 9:  a981f75 =  9:  a981f75 <errno.h> is guaranteed by ISO C
 -:  ------- > 10:  90b5f2d <limits.h> and INT_MAX are guaranteed by C89
 -:  ------- > 11:  3d86bb3 Use INT_MAX directly

@alejandro-colomar
Copy link
Author

alejandro-colomar commented Jan 25, 2024

@madler Do you have any thoughts about this patch set?

@madler
Copy link
Owner

madler commented Jan 25, 2024

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.

@alejandro-colomar
Copy link
Author

alejandro-colomar commented Jan 26, 2024 via email

@madler
Copy link
Owner

madler commented Jan 26, 2024

I may eventually apply them, once I look at them more closely.

@Neustradamus
Copy link

@gvollant: Have you seen this PR?

@alejandro-colomar alejandro-colomar force-pushed the stdc branch 2 times, most recently from fd93fe7 to 2f076dd Compare December 8, 2024 19:55
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 <alx@nginx.com>
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 <alx@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
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 <alx@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
And ironically, the only file that was calling this function, was also
including <limits.h> unconditionally.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
gz_intmax() is just a wrapper around INT_MAX.  Use the real thing.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
@AraHaan
Copy link
Contributor

AraHaan commented Dec 9, 2024

Most people still build under older compilers today. Let's use Elsword (the game) for example. They use zlib along with a handful of other dependencies and STILL compiled using Visual Studio 2010 until recently when their client side shipped the v140 VC runtime dll files. Also for the longest of time (according to an anonymous source) their server side was built with Visual Studio 2003 from before the /MP switch was added to the compiler. It is also likely that some C89 features were absent then as well but I do not know for sure.

Also, with Windows as an example they also for the longest of times built the kernel itself using a version of an older compiler as well because developing operating systems can sometimes be a pain to have binary compatible builds for older systems (that was before Windows 11 came though).

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

Successfully merging this pull request may close these issues.

6 participants