1

Im getting the error "Segmentation fault (core dumped)" when I run this program. I am new to c programming so its probably something stupid but i cant figure it out.

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

void swap(char *x, char *y){
    char temp = *x;
    *x=*y;
    *y=temp;
}

char** getPermutations(char *string){
    int length = strlen(string);
    int startIndex = 0;
    int endIndex = length - 1;

    if(startIndex == endIndex){
            printf("%s\n", string);
    }else{
            for(int j = startIndex; j<=endIndex; j++){
                    swap((string + startIndex),(string + j));
                    getPermutations(string);
                    swap((string+startIndex),(string+j));
            }  
    }    
}

int main(int argc, char *argv[]){
     if(argc>2){
            printf("\nToo Many arguments\n");
            exit(0);
    }else{
            printf("%s",argv[1]);
            char * str = malloc(strlen(argv[1]) + 1);
            strcpy(str,argv[1]);
            getPermutations(str);
    }
}
  • don't forget to free the allocated data at the end using `free()` – Cherubim Sep 21 '16 at 04:39
  • when compiling, always enable all the warnings, then fix those warnings that are output by the compiler. (for `gcc`, at a minimum use: `-Wall -Wextra -pedantic` I also use: `-Wconversion `std=gnu99` ) – user3629249 Sep 22 '16 at 15:17
  • for ease of readability and understanding: 1) separate code blocks (for, if, else, while, do...while, switch, case, default) via a blank line 2) consistently indent the code. never use tabs for indenting as every wordprocessor/editor has the tab stops/tab width set for personal preference. Suggest using 4 spaces for each indent level as that is wide enough to be visible even with variable width fonts and still allows for many indent levels across the page – user3629249 Sep 22 '16 at 15:24
  • error messages 'should' be sent to `stderr`, not `stdout` so when writing an error message, use: `fprintf( stderr, ........);` rather than `printf( ....... );` When finding a problem with the command line arguments, output a `usage` statement, similar to: `fprintf( stderr, "USAGE: %s ", argv[0] );` The current output does not tell the user anything about what the command line should be. Never access beyond `argv[0]` without first checking `argc` to assure that such argument actually exists – user3629249 Sep 22 '16 at 15:28
  • when calling most system functions, for instance `malloc()`, always check the returned value to assure the function was successful. – user3629249 Sep 22 '16 at 15:31
  • the function: `strlen()` returns type: `size_t` not `int`, so the variable `length` should be declared as `size_t` not `int` – user3629249 Sep 22 '16 at 15:33
  • regarding the code block beginning with: `for(int j = startIndex; j<=endIndex; j++){`, In C, indexes start at 0 and continue to `strlen(string) -`. So the code will access beyond the end of the input string buffer, resulting in undefined behavior that can/will lead to a seg fault event. – user3629249 Sep 22 '16 at 15:37
  • the function: `getPermutations()` does not return anything and the calls to that function do not use any returned value, so the signature for the function should be: ` void getPermutations(char *string)` – user3629249 Sep 22 '16 at 15:40

2 Answers2

2

Your issue is that getPermutations calls itself endlessly. You need to pass something extra to it so it can know when to stop. As is, it just calls itself over and over until you have a stack overflow.

Also, you have getPermutations setup to return a char**, but then you never return anything. So that's odd too.

michalska7
  • 33
  • 4
  • what else would I send to it? – Tyler Joseph Dominick Sep 21 '16 at 04:45
  • @TylerJosephDominick You might give it an initial value for `startIndex`. Then everytime you call it you do it like so: `getPermutations(startIndex + 1, string);` I have no idea if that does what you want, but at least it wouldn't run forever. Have you used recursion before? – michalska7 Sep 21 '16 at 04:54
  • I'm in my 3rd semester as a computer science student, I know about recursion and I'm pretty familiar with java programming but the class I'm taking now is all in c programming and so I'm pretty much lost with the whole memory allocation thing. Not to mention every time I ask my professor to look at my code he tells me to google and research the answer myself. Basically my assignment is to create a program in c that returns a list of permutations from a string argument. – Tyler Joseph Dominick Sep 21 '16 at 04:58
1

My suggestions:

  1. Change the return type of the function to void since it does not return anything.

  2. Change the name of the function to printPermutations since it just prints the permutations.

  3. Provide a way to end the recursion. Pass startIndex as an argument.

  4. Pass the length of the string so you don't compute it every time the function is called recursively.

  5. Change the comparison operator to >= to account for zero length strings.


void printPermutations(char *string, int startIndex, int length){
   int endIndex = length - 1;

   if(startIndex >= endIndex){
      printf("%s\n", string);
   }else{
      for(int j = startIndex; j<=endIndex; j++){
         swap((string + startIndex),(string + j));
         printPermutations(string, startIndex+1, length);
         swap((string+startIndex),(string+j));
      }  
   }    
}

and call it with:

printPermutations(str, 0, strlen(str));
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • @R Sahu I would make those changes but my assignment is as follows: "Use C code to write a function (a part of an API) with the signature char **getPermutations(char *), which, when given a string, returns all the permutations of the input string." I was simply attempting to print them one at a time to get started and hopefully see I was on the right track – Tyler Joseph Dominick Sep 21 '16 at 05:11
  • 1
    @TylerJosephDominick, that will require some learning on your part. You have to figure out what is needed to allocate an array of pointers and how allocate memory for strings that those pointers point to. It's not hard but it can be error prone. Good luck. – R Sahu Sep 21 '16 at 05:17
  • @R Sahu Thank you, do you have any recommendations of good resources to find the information I'm looking for? I feel completely lost. – Tyler Joseph Dominick Sep 21 '16 at 05:19
  • @TylerJosephDominick, http://stackoverflow.com/a/27883252/434551 and http://stackoverflow.com/a/9857578/434551. – R Sahu Sep 21 '16 at 05:25