Segmentation Fault

[FrostWraith]

OK, i am compiling using gcc on Ubuntu Feisty Fawn.  It compiles successfully with no errors or warnings. When run though, it throws Segmentation fault (core dumped).  Could someone point me in the right direction? Critique as well.

Code Select Expand

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

char* str_reverse(char* strmessage);

int main()
{

  char* mes = "this is the message that will be displayed backwards";
  char* rev;
  rev = str_reverse(mes);

  printf ("%s", rev);
  return 0;
}

char* str_reverse(char* strmessage)
{
  int i, j;
  char* temp, * buffer, * output;

    sprintf(buffer, "%s",strmessage);

    j=strlen(buffer)-1;

    for (i=0, j; i < j;i++, j--) {
      temp[i]=buffer[j];
    }

  strcpy(output, temp);

  return output;
}

[K]

So you have char* output, which is not initialized, and you copy a bunch of bytes to it... you do the same thing with "buffer" and "temp."  this might explain why your program segfaults.  You should also not try to return a pointer to a local variable.

You should google for some tutorials on string handling and/or pointers in C, because there are a lot really fundamental problems with your code.

[FrostWraith]

Dont think I haven't.  I just have no idea what to do in this situation.

[l2k-Shadow]

Dont think I haven't.  I just have no idea what to do in this situation.

He told you what to do, your problem is you are defining bunch of pointers without reserving a free section of memory for them, you may be putting stuff into somewhere where you shouldn't and hence your program crashes.

[MyndFyre]

For example, in str_reverse, you declare:

Code Select Expand


char* temp, * buffer, * output;

However, you never initialize these variables.  What this will do is increment the stack (because these locals will live on the stack), but since you never set them to a value, they don't point to anything in particular (they're "wild") -- they'll point to the address indicated by where they live on the stack.

To allocate a buffer, you might do something like:

Code Select Expand


char* temp = (char*)malloc(sizeof(char) * 120); // allocates a 120-character string on the heap
char buffer[120]; // allocates a 120-character string on the stack

Allocating the buffer on the stack makes memory management simpler (you don't need to clean up the variable later - it will be done when the function exits), but it'll also be invalid once the function exits.  Allocating the buffer on the heap makes the string available after the function exits, at the expense of the fact that you've got to free() it later.

Allocating a value on the stack can have its advantages.  For example, consider the following method:

Code Select Expand


char* str_reverse(char* input, char* output)
{
    if (output == NULL || input == NULL)
      exit(1);

    char buffer[120];
    // reverse algorithm here
    strcpy(output, buffer);
    return output;
}


Allocating the buffer on the stack is easy and quick because the function requires all calling functions to pre-allocate memory (a method which I find is most effective IMO) so they know they need to free it.

Alternatively, you can allocate the buffer on the heap and avoid too many parameters:

Code Select Expand


char* str_reverse(char* input)
{
    if (input == NULL)
        exit(1);

    char* buffer = (char*)malloc(sizeof(char) * 120);
    if (buffer != NULL)
    {
        // reverse algo here
    }
    return buffer;
}


Definitely look into tutorials on pointers and string handling in C.  I'd highly recommend the C for Dummies books (by Dan Gookin, the guy who wrote DOS for Dummies) - they're excellent.

[FrostWraith]

OK, thanks a lot, got it working, somewhat...

It only is displaying about half of the message in the terminal (sdrawkcab deyalpsid eb lli).

NEVERMIND!  I was being retarded in my loop.