[BUG]: VLA not supported in MSVC

[BatMW]

CThreads commit

5deaa64

Description

The function cthreads_error_string does not compile on any compilers that do not support variable-length array (VLA).
VLAs are part of the C99 standard but seems to be only supported by GCC and Clang (cppreference.com).

suggested solutions

• static size array using #define CTHREADS_ERROR_MSG_SIZE [Some reasonable size]
• malloc based on size_t length
• If you want to keep VLA for GCC and Clang and still support other compilers:

#if defined(__GNUC__) || defined(__clang__)
    #define VLA_SUPPORTED 1
#else
    #define VLA_SUPPORTED 0
#endif

//Implement 2 functions based on if VLA is supported or not

Steps to reproduce

Compile CThreads using MSVC.

Example code

> cl /TC /W4 /DCTHREADS_DEBUG test.c cthreads.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.42.34436 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

test.c
C:\prog\CThreads\cthreads.h(460): error C2057: expected constant expression
C:\prog\CThreads\cthreads.h(460): error C2466: cannot allocate an array of constant size 0
cthreads.c
C:\prog\CThreads\cthreads.h(460): error C2057: expected constant expression
C:\prog\CThreads\cthreads.h(460): error C2466: cannot allocate an array of constant size 0
cthreads.c(465): error C2057: expected constant expression
cthreads.c(465): error C2466: cannot allocate an array of constant size 0
Generating Code...

//Line 460 in cthreads.h
size_t cthreads_error_string(size_t length, char buf[length], int error_code);

Confirmations

• My environment meets the minimum requirements.
• I have verified that this is not a duplicate issue.

[ThePedroo]

That's really weird, as FrequenC has support for MSVC and didn't have it. Probably its compiling flags.

In any case, like the design of the project, it focus on being C89 compatible, so we must remove it completely (yes, it was a mistake). To ensure we're still keeping consistency in the API for all reasonable enough compilers, we can ask for the user to allocate the buffer themselves (be it dynamic or static) and just pass the pointer to the function, so that we memcpy to it.

I'm gonna PR that soon. Thank you.

[0xfadead]

Hi, I'm the author of this function and bug, sorry for the inconvience.

I made a PoC for a fix on godbolt:
https://godbolt.org/z/cW48c7Edv

[ThePedroo]

In my opinion, there aren't benefits on making it change the parameters based on those things, when compared to just asking for the pointer of a buffer and it's length. (https://godbolt.org/z/3M8ffMPPK)

[ThePedroo]

d129bfb this should fix it

[0xfadead]

In my opinion, there aren't benefits on making it change the parameters based on those things, when compared to just asking for the pointer of a buffer and it's length. (https://godbolt.org/z/3M8ffMPPK)

It gives compiler warnings when passing an array that's too short.

[ThePedroo]

Should be fine in 10f41f2, or else I need details

[0xfadead]

Should be fine in 10f41f2, or else I need details

[ThePedroo]

I mean, you're telling a 16-byte region is 32-byte long, so indeed it's correct

[0xfadead]

I mean, you're telling a 16-byte region is 32-byte long, so indeed it's correct

That's the point, the unsafe function didn't warn.

[ThePedroo]

That's VLA though, and CThreads is C89, although not explicitly said. The only reason I didn't see this before it was because that function was wrapped around rwlock, which in CPools Makefile CC flags, it is not supported.

EDIT: And with gcc, you can use -fanalyzer

[0xfadead]

That's VLA though, and CThreads is C89, although not explicitly said. The only reason I didn't see this before it was because that function was wrapped around rwlock, which in CPools Makefile CC flags, it is not supported.

EDIT: And with gcc, you can use -fanalyzer

AFAIK it's not, that's what the static is for. Also, I added the version checking around it. It only gets declared that way if the compiler supports it and it's at least C99 or if it is at least C11:

/*   Check for supported compiler, if it adheres to ISO C and if the version is >= C99 */
#if ((defined(__GNUC__) || defined(__clang__) || defined(__TINYC__)) && __STDC__ == 1 && __STDC_VERSION__ >= 199901L) \
/* Check if the compiler adheres to ISO C and if the version is >= C11 */
    || (__STDC__ == 1 && __STDC_VERSION__ >= 201112L)
#define CTHREADS_STATIC_VLA 1
#else
#define CTHREADS_STATIC_VLA 0
#endif

/**
 * Obtains the error code and writes at most `length` 
 * bytes of the associated message to `buf`.
 *
 * @param error_code Platform-specific error code. (See: `cthreads_error_code()`)
 * @param buf Buffer of `length` bytes and target of the error message
 * @param length Length of the provided buffer
 *
 * @return Number of bytes required to print the message + NULL-terminator
 */
#if CTHREADS_STATIC_VLA == 1
size_t cthreads_error_string(int error_code, char buf[static len], size_t length);
#else
size_t cthreads_error_string(int error_code, char* buf, size_t length);
#endif

EDIT: I personally would prefer it if the compiler warned me without having to add an extra flag which increases compile time.