Weird segfault involving popen

[liliafan]

Hi guys

It has been quite a long time since my last post, but I am having some very weird behaviour on a program I am writing and I was hoping to get a little community feedback ;)

I am writing a class that basically opens a pipe to ksh runs a script gets the results and passes it back to the main program, the weird thing is that as the program terminates it segfaults I know it is related to the class that includes popen but I can't figure out why it is happening:

This is the popen class:
cpp

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

#include "shell_interp.h"

using namespace std;

Shell_Interp::Shell_Interp()
{
}


Shell_Interp::~Shell_Interp()
{
}

int Shell_Interp::runshellscript(char *filename, std::string& returnme)
{
   FILE *kshfd;

   char line[200];  // Returned values shouldn't be more than char
   char *files_to_open;  
   
   sprintf(files_to_open, "/usr/bin/ksh %s", filename);
      
   if( (kshfd = popen(files_to_open, "r")) == NULL )
   {
      return 1; // Damn file descriptor is invalid
   }   
      
   fgets( line, sizeof(line), kshfd); // Get line not in a loop cause we know the output
   std::string return_result(line);   // Convert to string to return
   returnme = return_result;          // Return this
   
   pclose(kshfd); // Close file descriptor         
   return 0;
}

h

#ifndef SHELL_INTERP_H
#define SHELL_INTERP_H

#include <string>

/**
@author Ben Maynard
*/
class Shell_Interp{
public:
    Shell_Interp();
    ~Shell_Interp();
    int runshellscript(char *filename, std::string& returnme);
};

#endif

And this is the calling code in the main function (sorry it is very dirty at the moment it is just test code

Shell_Interp my_shell;     
     
     char *filename;
     std::string resulting;
     filename = "/home/work/test.sh";
     
     std::string getthisdude;
     
     int a = my_shell.runshellscript(filename, getthisdude);
          
     cout << "Ran shell script to get: " << getthisdude.c_str() << " return was " << a << endl;
     my_shell.~ Shell_Interp();

      return 0;

As it stand the returned string is exactly as expected, it runs everything before this point and everything after this point without a problem, but when it exits I get a segfault, if I remove the popen code no more segfaults so it is within this code here.

Any advice, help, pointers, questions, gratefully recieved.

Thanks

Ben

[Chainsaw]

files_to_open has no space reserved for it. You declared it as a char* but didn't 'new' it. So do one of these two things:

char* files_to_open = new char[ kMaxFileSize ]; // don't forget to delete it!
-or-
char files_to_open[ kMaxFileSize ];

[liliafan]

files_to_open has no space reserved for it. You declared it as a char* but didn't 'new' it. So do one of these two things:

char* files_to_open = new char[ kMaxFileSize ]; // don't forget to delete it! -or- char files_to_open[ kMaxFileSize ];

Damn you are good lol:

char files_to_open[ kMaxFileSize ];

Causes failure to compile:
shell_interp.cpp: In member function `int Shell_Interp::runshellscript(char*,
std::string&)':
shell_interp.cpp:31: error: cannot convert `char**' to `char*' for argument `1'
to `int sprintf(char*, const char*, ...)'
shell_interp.cpp:33: error: cannot convert `char**' to `const char*' for
argument `1' to `FILE* popen(const char*, const char*)'

However, char* files_to_open = new char[ kMaxFileSize ]; is perfect gets rid of the segfault completely.

I hate to impose further but any chance you could give me an indication as to why this happened? I would like to learn why this works for future reference instead of just that it does :D Thanks

Ben

[Chainsaw]

It sounds like you changed

sprintf(files_to_open, "/usr/bin/ksh %s", filename);

to

sprintf(&files_to_open, "/usr/bin/ksh %s", filename);

because that would be a char**, not a char*. Possible?

[liliafan]

It sounds like you changed

sprintf(files_to_open, "/usr/bin/ksh %s", filename);

to

sprintf(&files_to_open, "/usr/bin/ksh %s", filename);

because that would be a char**, not a char*. Possible?

Strangely enough no I didn't touch the sprintf() I just changed the declaration of file_to_open.

Ben