0

So, I've written this code below that is supposed to pass an array of strings to a function, which then sorts the array into alphabetical order. I know the way I've done it probably isn't pretty, but it is for school and I'm required to pass it to a function and make use of strcmp. I ran into some problems, but I managed to get all the compile errors sorted. Now, however, when I try to run the program, I get the error segmentation fault(core dumped). Can someone guide me to where I made my mistake? 

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

void sort(char *str[]);

int main()
{
    char *states[11] = {"Florida", "Oregon", "California", "Georgia"};

    sort(states);

    return 0;
}   

void sort(char *str[])
{
    int x, y;
    char alpha[11] = {0};

  for(x = 1; x < 4; x++){
    for(y = 1; y < 4; y++){
      if(strcmp(str[y - 1], str[y]) > 0){
        strcpy(alpha, str[y - 1]);
        strcpy(str[y - 1], str[y]);
        strcpy(str[y], alpha);
      }
    }
  }

  printf("\nThe states, in order, are: ");
  for(x = 0; x < 4; x++)
    printf("\n%s", str[x]);
}
Improve this question

2 Answers 2

Reset to default
2

You can't overwrite string literals which is what strcpy() would be doing, modifying string literals invokes undefined behavior, instead swap the pointers.

This

strcpy(alpha, str[y - 1]);
strcpy(str[y - 1], str[y]);
strcpy(str[y], alpha);

would work just fine like

alpha = str[y - 1];
str[y - 1] = str[y];
str[y] = alpha;

if you declare alpha as

char *alpha;

Also, notice that the size of the strings is not 11 in

char *states[11];

It's the number of pointers the array can hold. The pointers point to string literals, whose size is not really important in this case. The important thing is that the array contains pointers, and you can make pointers point somewhere else, but you can't change static memory like the one occupied by string literal.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I had not learned that much about string literals, so thank you very much for the information. Everything works as intended after making the suggested changes.
-1

Adding to the answer by iharob, your code should work if the lengths of all the string in states in 11. Your code crashes when it is trying to swap "Oregon" with "California". Since the length of "California" is 11 bytes and the length of "Oregon" is 7 bytes (including the null character), when you use strcpy to overwrite string array "Oregon" with "California" your buffer overruns and the program will core dump with signal 11. You could use the approach suggested by iharob or you could change the code like following:-

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

void sort(char str[][11]);

int main()
{
    char states[4][11] = {"Florida", "Oregon", "California", "Georgia"};

    sort(states);

    return 0;
}   

void sort(char str[][11])
{
    int x, y;
    char alpha[11] = {0};

  for(x = 1; x < 4; x++){
    for(y = 1; y < 4; y++){
      if(strcmp(str[y - 1], str[y]) > 0){
        strcpy(alpha, str[y - 1]);
        strcpy(str[y - 1], str[y]);
        strcpy(str[y], alpha);
      }
    }
  }

  printf("\nThe states, in order, are: ");
  for(x = 0; x < 4; x++)
    printf("\n%s", str[x]);
}

The output produced will be:- 

gaurav@ubuntu:~$ ./a.out 

The states, in order, are: 
California
Florida
Georgia
Oregon
Improve this answer

5 Comments

Your answer works, because your array is no longer an array of pointers to string literals, it works because your array is an array of arrays, which is writable. You can't write to the location where the string literals are stored, even if you write 1 byte to a destination with 1000000 bytes the program would invoke undefined behavior.
Hi, I would think 2-d array of characters hence modifiable. I do not understand " You can't write to the location where the string literals are stored, even if you write 1 byte to a destination with 1000000 bytes the program would invoke undefined behavior.". Can you clarify more? I was suggesting an alternate solution using strcpy.
Your answer seems to suggest that the issue is because the destination strings are too small, which is not true, that would be another problem, and using strcpy() in this situation is actually very very inefficient. Because you already used strcmp() and using strcpy() will iterate again through the while string.
I will like to refer to the problem statement "Passing an array of strings to a function-segmentation fault". In my post, I pointed out why the crash is happening and what is the possible workaround for the crash. Your solution provides a different way of achieving the same sort behavior. Certainly your solution is more efficient in terms of pointer swaps whereas the submitter is performing string copies using strcpy. Can you please tell me why my assumption "the destination strings are too small, which is not true" is wrong? Have you analyzed the backtrace of the core dump?
The following statement is fundamentally wrong "Adding to the answer by iharob, your code should work if the lengths of all the string in states in 11", so you can see why I say that your answer is wrong too, even though the code you posted obviously works.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.