Visual Studio Code - Why does this code work as expected if I use Visual C(Visual Sudio 2022) but doesn't if I use gcc 8.2?

OldCrow
July 1, 2023
161 views
2 votes
3 Answers

I am new to C and still learning(currently doing some "LeetCode" challenges). I wrote a function that should return the longest common prefix in an array of strings. I tested with a couple different values and all works fine. But a string array {"aaa", "aa", "aaa"} will result in "aa" (Correct) in Visual Studio but "aaa" in the gcc version and I do not understand why.

Here is my code:

#include<stdlib.h>
#include<stdio.h>
#include<string.h>

char* longestCommonPrefix(char** strs, int strsSize) {

    char* result = calloc(200, sizeof(char));
    if (result == NULL) return NULL;
    size_t result_length = strlen(strs[0]);
    memcpy(result, strs[0], result_length);
    char* result_end = result + result_length + 1;


    for (size_t i = 1; i < strsSize; ++i) {
        char* ptr_res = result;
        char* ptr_cur = strs[i];

        while (*ptr_res == *ptr_cur && *ptr_res > '' && *ptr_cur > '') {
            ++ptr_res;
            ++ptr_cur;
        }

        result_end = ptr_res;
    }
    *result_end = '';
    return result;
}


int main(int argc, char* argv[]) {
    char* a[3];
    a[0] = "aaa";
    a[1] = "aa";
    a[2] = "aaa";

    char* b = longestCommonPrefix(a, 2);
    printf(b);
    free(b);
}

Tags: c-strings c#pointers string undefined-behavior

Answers

- Chris
- July 2, 2023 at 12:15 am
- 0 votes
0
There are two issues in your code which jump out.

First off, you’ve called longestCommonPrefix with 2 rather than 3 being the length of the input array.

Secondly, within your loops, you never actually modify result by writing a new null terminator. You only dereference result_end and assign the null terminator to it after the last for loop iteration. This has the effect of only considering the first string in your array which is copied into result and what it sees as the "last" string in the array based on the length you pass in.

If the function gets called with 2 you print "aa". If you call it with 3 it considers the last string literal "aaa" and finds that between "aaa" and "aaa", "aaa" is the longest common prefix.

Cleaning a few other things up:
```
char* longestCommonPrefix(char **strs, int strsSize) {
    char *result = calloc(200, sizeof(char));
    if (result == NULL) return NULL;

    size_t result_length = strlen(strs[0]);
    memcpy(result, strs[0], result_length);

    for (size_t i = 1; i < strsSize; ++i) {
        char *ptr_res = result;
        char *ptr_cur = strs[i];

        while (*ptr_res == *ptr_cur && *ptr_res && *ptr_cur) {
            ++ptr_res;
            ++ptr_cur;
        }

        *ptr_res = '';
    }

    return result;
}
```
Login or Signup to reply.

- EricPostpischil
- July 2, 2023 at 12:21 am
- 0 votes
0
This is wrong:
```
char* result_end = result + result_length + 1;
```
result_length contains the number of characters in strs[0] before the null character, and result_end is used to set the null character later, so it should point to where the null character will go:
```
char* result_end = result + result_length;
```
Then this code:
```
while (*ptr_res == *ptr_cur && *ptr_res > '' && *ptr_cur > '') {
```
allows the loop to progress until the end of the string currently in the result buffer or the current string it is being compared to. That can be further than the longest common prefix established so far. The loop should be limited to the length of the longest common prefix:
```
while (*ptr_res == *ptr_cur && ptr_res < result_end) {
```
Observe that testing for null/non-null characters is not needed. If a null character is encountered in ptr_cur, then either *ptr_cur differs from *ptr_res or we have reached the end of the string in result, which result_end already points to (initially) or before (if it was updated from a prior comparison).

Also *ptr_res > '' is an incorrect way to test for a non-null character, as character values can be negative. *ptr_res != '' is a proper test.

printf(b); is hazardous, as it is prone to misbehavior if b is mishandled. Also, it is generally preferable to terminate program output with a new-line character. Use puts(b) or printf("%sn", b);.
Login or Signup to reply.

- VladfromMoscow
- July 2, 2023 at 12:25 am
- 0 votes
0
For starters you declared an array of 3 pointers to string literals
```
char* a[3];
a[0] = "aaa";
a[1] = "aa";
a[2] = "aaa";
```
but it is not clear why you are passing the second argument of this call
```
char* b = longestCommonPrefix(a, 2);
```
equal to 2 instead of 3.

Another problem is using the magic number 200 in the initializer of this declaration
```
char* result = calloc(200, sizeof(char));
```
within the funcion longestCommonPrefix.

In general the following statements
```
size_t result_length = strlen(strs[0]);
memcpy(result, strs[0], result_length);
```
can result in undefined behavior because a string pointed to an element of the array can be greater than the magic number 200.

And the dynamically allocated array pointed to by the pointer result after storing characters by means of calling memcpy does not contain a string.

This memory allocation entirely does not make sense because the passed array can contain a pointer to a null string.

Also the initializer of the variable result_end
```
char* result_end = result + result_length + 1;
```
also does not make snese and again can invoke undefined behavior due to the statement
```
*result_end = '';
```
because the pointer result_end does not point tp the memory after the last element of the copied string. That is the array will contain incorrect data. At least you should write
```
char* result_end = result + result_length;
```
And this while loop
```
    while (*ptr_res == *ptr_cur && *ptr_res > '' && *ptr_cur > '') {
        ++ptr_res;
        ++ptr_cur;
    }
```
can leave the content of the dynamically allocated array reuslt unchanged if the string pointed to by the pointer ptr_cur is shorter than the already stored sequence of characters pointed to by the pointer result.

And teh for loop is inefficient because there is no check that an empty string was already encountered.

Pay attention to that the function is unsafe because the second parameter can be a non-positive value.

And even this call of printf
```
printf(b);
```
in general can invoke undefined behavior if the common prefix contains symbols that form a format specification.

At first you need to find the length of the common prefix. And only afyter thst to allocate an array of the required size and copy the prefix as a string in the array.

The function and your program in whole should look the following way
```
#include <stdio.h>
#include <string.h>
$include <stdlib.h>

char * longestCommonPrefix( char **strs, size_t strsSize )
{
    size_t commonPrefixSize = 0;
    char *result = NULL;

    if (strsSize != 0)
    {
        commonPrefixSize = strlen( strs[0] );

        for (size_t i = 1; commonPrefixSize != 0 && i < strsSize; i++)
        {
            size_t n = 0;

            while (strs[i][n] != '' && strs[i][n] == strs[0][n]) ++n;

            if (n < commonPrefixSize) commonPrefixSize = n;
        }

        result = malloc( commonPrefixSize + 1 );

        if (result != NULL)
        {
            result[commonPrefixSize] = '';
            memcpy( result, strs[0], commonPrefixSize );
        }
    }

    return result;
}

int main( void )
{
    char *strs[] = { "aaa", "aa", "aaa" };
    size_t N = sizeof( strs ) / sizeof( *strs );

    char *commonPrefix = longestCommonPrefix( strs, N );

    if (commonPrefix != NULL)
    {
        printf( ""%s"n", commonPrefix );
    }

    free( commonPrefix );
}
```
The program output is
```
"aa"
```
Login or Signup to reply.