Unable to return string properly in C++ for some range

Question

I've made one C++ program using pointers and functions to return the reverse of a string. When the range char p[] in the function rev is low like 25 (less than. 145), the output is like ►↕☻ ☺♠♥, it doesn't reverse, when the range is higher than 145, it works fine in gcc, for Borland TurboC, the minimum range must be 65 otherwise the program prints strange values instead of reversing it.

#include<iostream>
#include<conio.h>
#include<string.h>
#include<stdio.h>
using namespace std;
int main()
{
  char str[20];
  int  l;
  char* p1;
  //clrscr();
  gets(str);

  l=strlen(str);
  char* rev(char*,int);
  p1=rev(str,l);
  puts(p1);
  return 0;
  //getch();
}

char * rev(char* a,int l1)
{
  char p[25]; // HERE: higher than 146 is working
  int c=0;
    for(l1=l1-1;l1>=0;l1--)
    {
       p[c]=a[l1];
       c++;
    }
  p[c]=NULL;
  return(p);
}

Undefined behaviour. If you are not able to use raw arrays and pointers properly, don´t do it, but use `std::string`. (Btw., don´t play around with SO´s editor buttons) — deviantfan, Jun 27 '15 at 11:18
but using strcpy() function to copy the pointer p1 to a string and displaying is working fine — Tuesday, Jul 02 '15 at 10:39
So what? UB doesn´t mean has to crash everytime, but it can. — deviantfan, Jul 02 '15 at 11:47

score 0 · Answer 1 · answered Jun 27 '15 at 11:31

0

When you declare

char p[25]

in function rev is only declared locally. When you'll reach the end of function, all of the local variables will be deleted (including array p). The easiest way to fix this is to change char p[25] to:

char *p = new char[25]

but beware, this might cause memory leaks if you won't free memory when you won't need it any more. I'd recommend you use std::string or take a look at some built in functions like strrev in string.h.

answered Jun 27 '15 at 11:31

Jaka Konda

1,385
3
13
35

but using strcpy() function to copy the pointer p1 to a string and displaying is working fine, thanks for answering – Tuesday Jul 02 '15 at 10:41

Vlad from Moscow · Accepted Answer · 2015-06-27T11:45:39.050

In function rev array char p[25]; is a local object of the function. After exiting the function it will not be alive and in general can be destroyed, that is the memory occupied by it can be overwritten by other object or function.

So the returned pointer to the first character of this array is invalid and the program has undefined behaviour.

It is unclear why the array in the function is defined with the size equal to magic number 25. If the second argument passed to the function will be greater than 25 then again there will be a problem because the memory will be overwritten beyond the array.

Also the design of the function is not good. Either the function reverses a string "in place" or copy in the reverse order the sourse array in the destination array and in this case the destination array has to be a function parameter.

And it is a bad idea to use C IO functions in C++ program. Moreover function gets is unsafe and is not supported any more by the C Standard.

The function can look the following way

#include <iostream>
#include <cstring>

char * reverse_copy( char *s1, const char *s2 )
{
    size_t n = std::strlen( s2 );
    size_t i = 0;

    for ( ; i < n; i++ ) s1[i] = s2[n - i - 1];
    s1[i] = '\0';

    return s1;
}

int main() 
{
    const size_t N = 20;
    char str1[N];
    char str2[N];

    std::cin.getline( str2, N );

    std::cout << str2 << std::endl;
    std::cout << ::reverse_copy( str1, str2 ) << std::endl;
}

If to enter phrase

Hello, Priyal Kumar

then the program output will be

Hello, Priyal Kumar
ramuK layirP ,olleH

thanks, :) but but using strcpy() function to copy the pointer p1 to a string and displaying is working fine — Tuesday, Jul 02 '15 at 10:41
@Priyal Kumar It is simply a bad approach. In fact you copy the string twice because the reverse operation is also some sort of a copy operation. And moreover these two operations are not logically connected. Somebody can forget that he need at first to copy the string and only then to call the reverse function. — Vlad from Moscow, Jul 02 '15 at 11:35

Unable to return string properly in C++ for some range

2 Answers2