0

I'm trying to write a C function to reverse a passed in C style string (ie char *) and return the char pointer of the reversed string. But when I run this in VS2012, nothing is printed in terminal and "main.exe has stopped working" msg shows up.

#include <stdio.h>
#include <string.h>
char * rrev_str(char * str )
{
    char *revd_str=""; //I tried char revd_str []="" error: stack around "revd_str" is corrupted
    int i,r;
    int str_len=strlen(str);
    for (i = str_len-1, r=0; i >=0; i--,r++)
    {
        revd_str[r]= str[i];
    }
    return revd_str;
}

int main(int argc, char* argv[])
{
   char str1 [] ="STEETS";
   char str2 [] ="smile everyday!";

   //reverse "chars" in a C string and return it
   char * rev_string=rrev_str(str1);
}
jerryh91
  • 1,777
  • 10
  • 46
  • 77
  • http://stackoverflow.com/questions/8784099/why-does-this-small-c-program-crash/8784124#8784124 Same problem as this one .. – JRK May 17 '13 at 18:01
  • Run-Time Check Failure #2 - Stack around the variable 'revd_str' was corrupted. – jerryh91 May 17 '13 at 18:24

3 Answers3

2

The problem here is three fold. First you aren't allocating enough space for the reversed string, and secondly you are returning a pointer to a local variable in rrev_str(), and thirdly you're modifying a string literal. You need to allocate space for revd_str on the heap:

char * rrev_str(char * str )
{   
    int i,r;
    int str_len=strlen(str);

    char *revd_str=malloc(str_len + 1); 
    memset(revd_str, 0, str_len + 1);

    for (i = str_len-1, r=0; i >=0; i--,r++)
    {
        revd_str[r]= str[i];
    }
    return revd_str;
}
mshildt
  • 8,782
  • 3
  • 34
  • 41
  • 2
    You'll need to assign the terminator as well, as malloc doesn't guarentee a zero'ed memory location. – Femaref May 17 '13 at 18:02
  • Thanks that worked. With 1 small syntax edit: memset's 3rd arg expects a variable and no other operation done with that var when passed thru. so instead of str_len+1, it expects a single variable storing this val. So I put str_len+1 in a temp_var and passed it to memset – jerryh91 May 17 '13 at 18:40
  • Yes it will work. But you need to free heap memory when it is not require. It may be memory leak if you not free it in future. – Navnath Godse May 17 '13 at 18:58
1

Problem:

You are accessing invalid memory address.
revd_str is pointing to literal constant string of length 1 and you are accessing it beyond the length which is invalid.

Solution:

  • Create char array of require length (statically or dynamically).
  • Reverse the given string.
  • Pass 2nd param as destination string
    syntax: char * rrev_str(char * src, char *dest);

Reverse the given string

char * rrev_str(char * str )
{
   int start = 0;
   int end = strlen(str) - 1;
   char temp;

    for (; start < end; start++ ,end--)
    {
        temp = str[start];
        str[start] = str[end];
        str[end] = temp;
    }
    return str;
}

int main(int argc, char* argv[])
{
   char string [] ="smile";

   //reverse "chars" in a C string and return it
   char * rev_string = rrev_str(string);

   printf("%s",rev_string);
}


Pass 2nd param as destination string

char * rrev_str(char * src, char *dest)
{
   int srcLength = strlen(src);
   int destLength = strlen(dest);
   int i;
   // Invalid destination string
   if (srcLength > destLength)
   {
        return NULL;
   }

   dest[srcLength] = '\0';
   srcLength--;
    for (i=0; srcLength >= 0;i++, srcLength--)
    {
        dest[i] = src[srcLength];
    }

 return dest;
}

int main(int argc, char* argv[])
{
   char string [] ="smile";
   char revString[20];  

   //reverse "chars" in a C string and return it
   char * rev_string = rrev_str(string, revString);

    printf("%s",rev_string);
}
Navnath Godse
  • 2,233
  • 2
  • 23
  • 32
0

What! you are doing..

char *revd_str=""; // Creating String Literal which can't be modified because they are read only  
char *revd_str[]=""; // Creating Char Array of Size Zero.

So Solution are

Either take reference of your string

char *revd_str = strdup(str);

Or create dynamic char array

char *revd_str = (char*) malloc (strlen(str)+1);

your program will run fine. logic is incorrect for reversing so modify it. A sample solution is given below

char * rrev_str(char * str )
{
    char *revd_str=strdup(str);
    int i;  // no need for extra 'int r'
    int str_len=strlen(str);
    for (i = 0; i < str_len/2; i++)
    {
        char temp = revd_str[i];
        revd_str[i]= revd_str[str_len - 1 -i];
        revd_str[str_len - 1 -i] = temp;
    }
    return revd_str;
}
Community
  • 1
  • 1
Jai
  • 1,292
  • 4
  • 21
  • 41
  • Or perhaps use `strdup` to get a duplicate of the same size. Also, `char *revd_str=str` does not create a copy of str, it overwrites str. – user1055604 Mar 16 '14 at 14:04
  • thank you for pointing it out. I was taken wrong it as change in given string. I have corrected it. – Jai Mar 19 '14 at 05:26