Pointers and string reverse function implementation

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;

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).