wrong output, can't understand why C

Question

I am a beginner in C programming and I built a function that recieves the string "HodHasharon,frozenYogurt,100". The function cuts the string into 3 pieces where every piece is a field of my City struct.

I want to put "HodHasharon" into City pcity.name (name of the city), "frozenYogurt" into pcity.popularFood (popular food) and number of residents (100) into pcity.residents.

When i am debugging the output in my function the output is correct but when i print from the main.c I got a concatenated string.

For example, when I print pcity.name I get "HodHashafrozenYod" instead of "HodHasharon" but if I do printf at my function printf->name I get the correct output of "HodHasharon"

What am I doing wrong?

struct of City:

typedef struct City
{
    char *name;
    char * popluarFood;
    int numberOfPeople;

} City;

the function:

City * cutCityData (char *singleLine)
{
    City* pcity=(City*)malloc(sizeof(City));
    int firstIndex=1;
    int endIndex=1;
    int checkItarion=0;
    while(endIndex<strlen(singleLine))
    {//while
        while (singleLine[endIndex] != ',')
        {//while2
            endIndex++;
        }//while2
        checkItarion++;
        char cityDetails[endIndex - firstIndex +1];
        memcpy(cityDetails,&singleLine[firstIndex], endIndex);
        cityDetails[endIndex - firstIndex] = '\0';
        if (checkItarion == 1) {
            pcity->name = (char *) malloc(cityDetails);
            strcpy(&(pcity->name), cityDetails);
            endIndex++;
            firstIndex = endIndex;
        }
        if (checkItarion == 2) {
            pcity->popluarFood = (char *) malloc(cityDetails);
            strcpy(&(pcity->popluarFood), cityDetails);
            endIndex++;
            firstIndex=endIndex;
            break;
        }
    }//while
    char cityDetails[strlen(singleLine) - firstIndex + 1];
    memcpy(cityDetails, &singleLine[firstIndex], sizeof(singleLine-1));
    int resdints=atoi(cityDetails);
    pcity->numberOfPeople=resdints;
    return pcity;
    }

from main:

City* pCity=cutCityData(singLine);
printf("%s\n", &(pCity->name));

Read https://ericlippert.com/2014/03/05/how-to-debug-small-programs/ for some tips on debugging your code. If you still need help, please [edit] your question to include a [mcve]. — Code-Apprentice, Nov 13 '18 at 16:52
[don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Nov 13 '18 at 16:52
You can use `strchr()` to search for a `,` instead of writing your own loop. — Barmar, Nov 13 '18 at 17:00
Array indexes start at `0`. Why are you setting `firstIndex` to `1`? — Barmar, Nov 13 '18 at 17:01
pCity-> without & i got : Process finished with exit code 139 (interrupted by signal 11: SIGSEGV) — , Nov 13 '18 at 17:02
" HodHasharon,frozenYogurt,100" this is the input , the first char is "\t" — , Nov 13 '18 at 17:05
Please refer to the first comment. Read both the "how to debug" and the MCVE link. Really. Do. Most problems go away on their own with those two pieces of advice alone... — DevSolar, Nov 13 '18 at 17:07

score 1 · Answer 1 · answered Nov 13 '18 at 17:13

&(pcity->name) is the address of the pointer variable. You want to copy the string to the memory that it points to, not copy over the pointer. So change:

strcpy(&(pcity->name), cityDetails);

to

strcpy(pcity->name, cityDetails);

You're also giving the wrong argument to malloc(). cityDetails is an array, but the argument should be the number of bytes you want to allocate. So change

pcity->name = (char *) malloc(cityDetails);

to:

pcity->name = malloc(strlen(cityDetails) + 1);

These changes also need to be made for the code that fills in pcity->popularFood as well.

This is wrong:

memcpy(cityDetails, &singleLine[firstIndex], sizeof(singleLine-1));

singleLine is a pointer, so sizeof(singleLine-1) is the number of bytes in a pointer, not the length of the string. This should be:

memcpy(cityDetails, &singleLine[firstIndex], endIndex + 1);

wrong output, can't understand why C

1 Answers1