Why does this code bit work in clion but doesn't in VS

Question

Hi I copied the following code from my linux machine with clion running. But in VS on Windows it seems to cause problems

entry_t* find_entry( char* n ) 
{
    // TODO (2)
    int x = strlen(n);
    char str[x];

    for (size_t i = 0; i < strlen(str); i++)
    {
        str[i] = toupper(n[i]);
    }

    n = &str;

    for (size_t i = 0; i < list_length; i++)
    {
        if (strcmp(n, name_list[i].name) == 0)
        {
            return &name_list[i];
        }
    }
}

VS underlines the x in char str[x]; before the statement do find x was in the brackets of str. I thought finding the length first in another variable would solve the problem

VS give the following error

Schweregrad Code Beschreibung Projekt Datei Zeile Unterdrückungszustand Fehler (aktiv) E0028 Der Ausdruck muss einen Konstantenwert aufweisen. Names.exe - x64-Debug C:\Users\Eyüp\source\repos\09\main.c 102

trying my best to translate it -> Error(active) E0028 Statement needs to be a constant value

Visual Studio doesn't support Variable Length Arrays when using the Microsoft compiler. [How to use Visual Studio as an IDE with variable length array(VLA) working?](https://stackoverflow.com/questions/58462881/how-to-use-visual-studio-as-an-ide-with-variable-length-arrayvla-working) — Retired Ninja, Dec 04 '22 at 17:58
That looks like horrible piece of code anyway :D Best option would be to just replace `str[x]` with `malloc` and remember to free it before `return`l Also if none matches, your code has undefined behaviour. — Antti Haapala -- Слава Україні, Dec 04 '22 at 18:01
`char str[x];` is not long enough and you are not adding the nul terminator. — stark, Dec 04 '22 at 18:02
@Venox do you mean correcting it gets the job done, or that you are a victim of *undefined behaviour* who is deceived into thinking the mistakes are 'ok'? — Weather Vane, Dec 04 '22 at 18:06
@Venox: Re “It gets the job done :)”: No, it does not. You might not have observed a problem, but that does not mean one is not present. Unseen by you, the `strcmp` is overrunning the array. In a different C implementation or in different circumstances in the same C implementation, that will cause a crash, data corruption, or other problems. Do not be snarky and dismiss problems with “:)”. Computers are machines, and they will not forgive or overlook errors. Learn to program correctly, and pay attention to the rules. — Eric Postpischil, Dec 04 '22 at 18:07

score 0 · Answer 1 · answered Dec 04 '22 at 18:09

Your code invokes undefined behaviour:

as you do not null terminate the string
you call strlen on not null terminated (and initially not initialized string)

The logic is also wrong.

entry_t* find_entry( const char* n ) 
{
    // TODO (2)
    size_t x = strlen(n);
    char str[x + 1];

    for (size_t i = 0; i <= x; i++)
    {
        str[i] = toupper((unsigned char)n[i]);
    }
    str[x] = 0;

    for (size_t i = 0; i < list_length; i++)
    {
        if (strcmp(str, name_list[i].name) == 0)
        {
            return &name_list[i];
        }
    }
    return NULL;
}

You need to return something if the sting was not found.

To use VLAs in VS How to use Visual Studio as an IDE with variable length array(VLA) working?

Antti Haapala -- Слава Україні · Accepted Answer · 2022-12-04T18:38:23.987

Variable-length arrays (i.e. arrays whose size is not known at compile-time) are not supported in MSVC because they don't care. Hence you need to use malloc and friends instead.

However that is not the only problem in your code: it has multiple undefined behaviours. Here is a suggested fix:

entry_t* find_entry( char* n ) 
{
    // return value of strlen is of type size_t, not int
    size_t x = strlen(n);

    // [x] was wrong, it needs to be [x + 1] for the null terminator!
    char *str = malloc(x + 1);

    // do not use strlen again in the loop. In worst case it does need 
    // to go through the entire string looking for the null terminator every time.
    // You must copy the null terminator, hence i <= x or i < x + 1
    for (size_t i = 0; i <= x; i++)
    {
        // the argument of toupper needs to be *unsigned char*
        str[i] = toupper((unsigned char)n[i]);
    }

    // why this did even exist? And it has type error anyway
    // n = &str;

    for (size_t i = 0; i < list_length; i++)
    {
        if (strcmp(str, name_list[i].name) == 0)
        {
            // need to free the str...
            free(str);
            return &name_list[i];
        }
    }

    // in both paths...
    free(str);

    // add default return value
    return NULL;
}

Why does this code bit work in clion but doesn't in VS

2 Answers2