Segmentation fault when copying string to

Question

Hit there!

Below is a little program I made today. It takes a table of strings, reverses all strings without reversing table, then sorts those reversed string, then reverses them back, and at last prints whole table.

I was trying really long to figure out why can't I copy 'slowo' string into place in table pointed in strcmp, but with no success. I would be happy if someone would find a way to fix Segmentation Fault in this case, but I really want to leave the method as it is below.

Thanks for your help! :)

EDIT By using debugger I determined that Segmentation Fault appears in strcpy, if that wasn't clear...

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

const int ROZMIAR=4;
char* nieposortowane[]={"basia", "zosia", "ala", "genowefa"};

int porownaj(char* a, char* b)
{
return strcmp(a,b);
}

void sortowanie(char** tablica, int N)
{
int zamienione, i;
char tmp;

do
{
    zamienione=0; 
    for(i=0;i<N-1;i++)
        if(porownaj(nieposortowane[i], nieposortowane[i+1])>0)
        {

            tmp=**(tablica+i);
            **(tablica+i)=**(tablica+(i+1));
            **(tablica+(i+1))=tmp;

            zamienione=1;
        }
}
while(zamienione);
}

void wypisz(char** tablica, int N)
{
int i=0;
for(i=0;i<N;i++)
    printf("%s\n", *(tablica+i));
}

void odwr(char** tablica, int N)
{
int i, ln, c;
int start, koniec;
char temp;

for(i=0;i<N;i++)
{
    ln = strlen(tablica[i]);
    char slowo[ln];
    strcpy(slowo,*(tablica+i));
    start=0;
    koniec=ln-1;
    for(c=0;c<(ln/2);c++)
    {
        temp =slowo[start];
        slowo[start]=slowo[koniec];
        slowo[koniec]=temp;
        start++;
        koniec--;
    }
    strcpy(*(tablica+i), slowo);
}
}

int main()
{
printf("Przed sortowaniem: \n");
wypisz(nieposortowane, ROZMIAR);

odwr(nieposortowane, ROZMIAR);
sortowanie(nieposortowane, ROZMIAR);
odwr(nieposortowane, ROZMIAR);

printf("Po sortowaniu babelkowym: \n");
wypisz(nieposortowane, ROZMIAR);

return 0;
}

Answer 1

AFAICS, you are trying to modify the strings in:

char* nieposortowane[]={"basia", "zosia", "ala", "genowefa"};

Those are string literals stored in read-only memory; any attempt to modify them (for example, by reversing them) will fail.

I reproduced the crash with your original code. This revision of the code doesn't crash; it goes into an infinite loop in your sort code instead. I've not debugged that part of your code.

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

const int ROZMIAR=4;
char basia[] = "basia";
char zosia[] = "zosia";
char ala[] = "ala";
char genowefa[] = "genowefa";
char* nieposortowane[]={basia, zosia, ala, genowefa};

int porownaj(char* a, char* b)
{
return strcmp(a,b);
}

void sortowanie(char** tablica, int N)
{
int zamienione, i;
char tmp;

do
{
    zamienione=0; 
    for(i=0;i<N-1;i++)
        if(porownaj(nieposortowane[i], nieposortowane[i+1])>0)
        {

            tmp=**(tablica+i);
            **(tablica+i)=**(tablica+(i+1));
            **(tablica+(i+1))=tmp;

            zamienione=1;
        }
}
while(zamienione);
}

void wypisz(char** tablica, int N)
{
int i=0;
for(i=0;i<N;i++)
    printf("%s\n", *(tablica+i));
}

void odwr(char** tablica, int N)
{
int i, ln, c;
int start, koniec;
char temp;

for(i=0;i<N;i++)
{
    ln = strlen(tablica[i]);
    char slowo[ln];
    strcpy(slowo,*(tablica+i));
    start=0;
    koniec=ln-1;
    for(c=0;c<(ln/2);c++)
    {
        temp =slowo[start];
        slowo[start]=slowo[koniec];
        slowo[koniec]=temp;
        start++;
        koniec--;
    }
    strcpy(*(tablica+i), slowo);
}
}

int main()
{
printf("Przed sortowaniem: \n");
wypisz(nieposortowane, ROZMIAR);

odwr(nieposortowane, ROZMIAR);
wypisz(nieposortowane, ROZMIAR);

sortowanie(nieposortowane, ROZMIAR);
wypisz(nieposortowane, ROZMIAR);
odwr(nieposortowane, ROZMIAR);
wypisz(nieposortowane, ROZMIAR);

printf("Po sortowaniu babelkowym: \n");
wypisz(nieposortowane, ROZMIAR);

return 0;
}

Sample output:

Przed sortowaniem: 
basia
zosia
ala
genowefa
aisab
aisoz
ala
afewoneg

I had to interrupt after that, but you can see that the strings are reversed successfully.

You'll need to sort out a more general solution; naming the individual arrays as I did is easy for a small fixed set, but not generally. One possibility would be to use strdup() to duplicate each string into allocated space:

enum { SIZE_NPSW = sizeof(nieposortowane) / sizeof(nieposortowane[0] };

for (i = 0; i < SIZE_NPSW; i++)
    nieposortowane[i] = strdup(nieposortowane[i]);

Answer 2

When allocating slowo, you should add 1 to the result of strlen when calculating the size to allocate. This is to accomodate the terminating null character, which is not included in a string's length (returned by strlen), but must be included in its total allocated size.