Pointers and string reverse function implementation

1. indent your code. Indentation should normally reflect control flow:

   char * reverse_string(const char *s1)
   {
       int count = 0;
       char *output;
       char *o;
       while (*s1 != '\0')
           s1++;
   
       s1--;
       output = new char[strlen(s1)];
       o = output;
   
       while (*s1)
           *output++ = *s1--;
   
       if (*s1 == '\0')
           cout << "yes";
       return o;
   }
   
   int main()
   {
       const char *s1 = "hello world";
       char *result;
       result = reverse_string(s1);
       cout << result;
   }
   

2. When you're manipulating strings in C++, prefer std::string over pointers to char. Likewise, prefer standard algorithms to duplicating their functionality. Using std::string, and a standard algorithm, your core function can reduce to something like this:

   std::string reverse_string(std::string in) {
       std::reverse(in.begin(), in.end());
       return in;
   }
   

3. Prefer to initialize variables rather than creating them uninitialized, then assigning a value later. For example, this:

   char *result;
   result = reverse_string(s1);
   cout<<result;
   

   ...would almost certainly be better off as:

    char *result = reverse_string(s1);
   

4. Avoid intermediate variables unless they actually contribute something useful. The sequence above would probably be just as well off being written as:

   std::cout << reverse_string(s1);
   

5. Avoid using namespace std;. Although you don't actually show this in your code, using cout without std:: indicates that you probably have it elsewhere in your code. If you use this at all, you generally want to restrict it to a relatively small scope (but it's generally better to avoid it completely).

6. If you insist on allocating space for the string "by hand" (i.e., directly invoking new char[len], be sure to include enough space for the NUL terminator. Right now you have:

   output = new char[strlen(s1)];
   

   ...which allocates space for the characters in the string, but no space for the zero-byte that signals the end of the string. Along with this, when you copy the data, you need to ensure that you put a zero-byte at the end of the result to signal where it ends. But, see point number 2 above--despite its (many) flaws, you're much better off using std::string, and avoiding this problem entirely.

7. A function should have a single intent. Right now, your reverse_string not only reverses its input, but also prints out "yes". This restricts that function to being using only under very specific circumstances. By strong preference, a function that reverses a string should do only that, without printing out anything (and the same applies in general, not just to this specific function).

The code which you have pasted is prone to segmentation fault and the behaviour is unpredictable. Sometimes, you may get the output "yes" because of the way the memory manager allocates memory. The unsafe operation in the code is due to the following loop.

while(*s1)
*output++ = *s1--;

The while loop may only break if the value pointed to at the location before the start of the string s1 is 0. But then, this memory location you should not access. If you are lucky, there will be a string (say s0) allocated just before s1 starts and since s0 is a null terminated string, *(s0 + len(s0)) = *(s1-1) = 0;

You should rather iterate s1 from left-right and fill up the output buffer from right-left shown as below.

    char *p;
    int len = strlen(s1);
    char* output = new char[len];

    for (p = s1; *p; p++) {
       output[len-1] = *p;
       len--;
    }
    return output;

I know what you're doing putting stars in the front and end of your functions, but as I tried to edit your code w/o *'s I was unsuccessful. All you had to do was copy and paste this into your main, but it takes 10 seconds to compile so your logic is getting into memory segmentation.

 char word[LEN], *x, *y;
 cout<<"Enter a word\n";
 cin>>word;

 char *result;
 result = reverse_string(word);
 cout<<result;

When I tried taking all the tokens out to have word passed in as a char, I'm not able to get the s1 to return the reverse value back.

You are clearly trying to use C-strings and the cat function because strcat put back together and then strcpy back into the argument c-string for the pass by reference to looping through the character array to make a backwards copy.

Here's a good example of deleting stuff w/o doing what you did above. Allowing less need for *s in main or in the function header calls.

#include <iostream>
#include <cstring>
#include <stdio.h>
#include <string.h>
#include <iostream>
#include <cctype>
using namespace std;
char removePunct(char[]);

int main()
{
int length;
string bob;
const int LEN = 64;
char word[LEN], x, y;
char *result;

cout<<"Enter a word\n";
cin>>word;
cout<<"You enter the word "<<word<<endl;

    x=removePunct(word);

    cout<<x;

    return 0;
    }

    char removePunct(char word[])
    {   // strtok example 

    char * pch;
    //int length=strlen(word);
    //printf ("Splitting string \"%s\" into tokens:\n",str);
    pch = strtok (word," ,'\' '.-/];''[");

     while (pch != NULL)
    {
    printf ("%s",pch);
    pch = strtok (NULL, " ,'\' '.-/];''[");

     word=pch;
     }
     }