Skip to content

Minor recommended change to defines.wn #401

@thegavaguy

Description

@thegavaguy

In defines.wn, the version of define.h for use in Windows builds of less,
the following is seen:

    259 /* Define HAVE_ERRNO if you have the errno variable */
    260 /* Define MUST_DEFINE_ERRNO if you have errno but it is not define
    261  * in errno.h */
    262 #define HAVE_ERRNO 1
    263 #ifdef MINGW
    264 #define MUST_DEFINE_ERRNO 0
    265 #else
    266 #define MUST_DEFINE_ERRNO 1
    267 #endif

The symbol MUST_DEFINE_ERRNO is only used in os.c. In the two uses in that
file it controls the following code:

#if MUST_DEFINE_ERRNO
    extern int errno;
#endif

The excerpt above from defines.wn says, in essence, that the declaration of
errno is not part of errno.h for non-MINGW builds. I would like to place
the following into evidence to show that this has not been true for a long time.

I have access to full installs of the following versions of Visual Studio, with
the indicated versions of the Microsoft C compiler:

Visual Studio 6.0 (1998)
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 12.00.8804 for 80x86
Copyright (C) Microsoft Corp 1984-1998. All rights reserved.

Visual Studio 2010 10.0 (2010)
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.40219.01 for 80x86
Copyright (C) Microsoft Corporation. All rights reserved.

Visual Studio 2022 17.0 (2022)
Microsoft (R) C/C++ Optimizing Compiler Version 19.36.32535 for x64
Copyright (C) Microsoft Corporation. All rights reserved.

For each of these, here is the relevant portion of errno.h supplied with the
indicated version:

VS 6.0 errno.h (excerpt)

/* declare reference to errno */

#if     (defined(_MT) || defined(_DLL)) && !defined(_MAC)
_CRTIMP extern int * __cdecl _errno(void);
#define errno   (*_errno())
#else   /* ndef _MT && ndef _DLL */
_CRTIMP extern int errno;
#endif  /* _MT || _DLL */

VS 2010 errno.h (excerpt)

/* Declare reference to errno */

#ifndef _CRT_ERRNO_DEFINED
#define _CRT_ERRNO_DEFINED
_CRTIMP extern int * __cdecl _errno(void);
#define errno   (*_errno())

errno_t __cdecl _set_errno(_In_ int _Value);
errno_t __cdecl _get_errno(_Out_ int * _Value);
#endif

VS 2022 errno.h (excerpt)

#if _CRT_FUNCTIONS_REQUIRED
    _ACRTIMP int* __cdecl _errno(void);
    #define errno (*_errno())

    _ACRTIMP errno_t __cdecl _set_errno(_In_ int _Value);
    _ACRTIMP errno_t __cdecl _get_errno(_Out_ int* _Value);

    _ACRTIMP unsigned long* __cdecl __doserrno(void);
    #define _doserrno (*__doserrno())

    _ACRTIMP errno_t __cdecl _set_doserrno(_In_ unsigned long _Value);
    _ACRTIMP errno_t __cdecl _get_doserrno(_Out_ unsigned long * _Value);
#endif // _CRT_FUNCTIONS_REQUIRED

So, for at least the last 25 years, the errno.h file provided with Microsoft's
C compilers has defined the symbol errno. In VC98 single-threaded compiles, it
was defined as an extern reference to an int. For multi-threaded compiles, it was
defined as a dereferenced pointer to int returned from a void function.

In later releases the C runtime was implicitly multi-threaded and so the 2010 and
2022 versions only declare a function-based implementation of errno.

Now here is the crazy thing: the extra declarations in os.c don't cause an error
at compile time, but it is mostly down to luck. The combination of

#if MUST_DEFINE_ERRNO
    extern int errno;
#endif

in os.c and

#define errno (*_errno())

in recent versions of errno.h yield

    extern int (*_errno())

which is a valid way to access errno and duplicates the existing declaration,
thus not triggering a compile-time error. However, it is misleading because the
necessary declaration is already present in errno.h and such a redefinition
might cause problems in the future.

Since errno.h in MINGW toolchains and Microsoft C compilers going back at least
25 years have valid declarations of errno, I would propose that the relevant
portion of defines.wn be modified to simply read:

    259 /* Define HAVE_ERRNO if you have the errno variable */
    260 /* Define MUST_DEFINE_ERRNO if you have errno but it is not defined
    261  * in errno.h */
    262 #define HAVE_ERRNO 1
    266 #define MUST_DEFINE_ERRNO 0
    267 <deleted>

Any questions or comments? Is this a good thing to change? Would this really need a PR?

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions