Memory Safe?

[matty3269]

Hi all,

I was just wondering weather you could tell me weather this function that i have wrote will be memory safe:

char* fnPipeCommand(const char* command)
{
vector<char> alloc;
char temp;
char* ret = NULL;
FILE *fp;

fp = popen(command, "r");
if(fp != NULL)
{
while((fread(&temp, 1, 1, fp)) != 0)
{
alloc.push_back(temp);
}

if((int)alloc.size() == 0)
return NULL;

ret = (char*)malloc(alloc.size());
for(unsigned int i =0; i < alloc.size(); i++)
{
ret[i] = alloc[i];
}
}
return ret;
}

I have had to write it like this becuase the popen call will return a FILE* but i can not do an fseek to find the size of the pipe so i decided to use a vector. Is there a better way to do this?

Thanks,
Matt

[Duthomhas]

In essentials you've got the right idea. You never close the pipe, though, and std::string is optimized for character strings while std::vector is not. You've also got a fencepost error.

#include <cstdio>
#include <cstdlib>
#include <string>
...

char* fnPipeCommand( const std::string& command )
  {
  std::FILE*  fp;
  std::string input;
  char*       result = NULL;

  fp = std::popen( command.c_str(), "r" );
  if (fp != NULL)
    {
    input.reserve( 1024 );  // see note below
    while (true)
      {
      int c = std::fgetc( fp );
      if (c == EOF) break;
      input.push_back( (char)c );
      }
    std::fclose( fp );

    if (!input.empty())
      {
      result = (char*)std::malloc( input.length() +1 );
      input.copy( result, input.length() );
      result[ input.length() ] = '\0';
      }
    }

  return result;
  }

I changed the argument type to a const string reference, because this is more flexible. It will take std::string and char* etc.

I'm also not sure why you are using malloc() instead of new, or simply returning a std::string, but I assume you have a reason.

On line 15 we reserved 1024 chars for use by the input string. This is so that fewer reallocations need be done as we increase the size of the string. This assumes that the pipe will return less than 1024 characters. You should reserve just a little over what you think the average length of the message from 'command' will be.

Hope this helps.

PS. Please use [code] tags.

[simo110]

To matty3269 :
Does your code run in the os based on posix.2 ?

I think you need try the one language as c or c++,don't mix the trait of the two language !
In posix system ,I sugest the c not c++! Just as the Linus's choose!

[Duthomhas]

Please don't start language wars in other people's threads. C++ is designed to mix well with C and there is no reason why it mustn't be used on POSIX systems.

[simo110]

I don't want to start any language war!
Just a advice in my experience of Linux programming!
Choose it or not!

[weaknessforcats]

I agree with simo110. This is not a language war. This is about mixing two different languages. C++ is not C and does not mix well with C. After 18 years with C++, I am convinced that knowing C is a big deteriment when it comes to learning C++. C++ and Java don't mix either even though they kinda look the same.

This thread is the typical experience: A mixture of C (char*, FILE*m,
fread(), malloc()) and C++ (vector<char>). Especially malloc(). Using
malloc() in C++is a death sentence.

Either the char*, FILE*, fread() and malloc() need to be removed and replaced with string, fstream, new) or the vector<char> needs to be removed and replaced with whatever code you need to manage a native array.

[Duthomhas]

Listen you idiots.

The OPs question has absolutely nothing to do with choice of language, except that he already knows he is mixing C into C++. Jumping up and down and telling him to forget C++ does not help; moreover, it is a waste of bandwith because nobody cares.

C++ is designed to mix with C.

You can believe it or not, but before you start spouting off about your piddly amount of "experience" you might want to read straight from the horse's mouth what the design goals of C++ are:
http://www.research.att.com/~bs/hopl-almost-final.pdf

About malloc(), I suspect that Marshall Cline has more knowledge about using it in C++ than you do:
http://www.parashift.com/c++-faq-lite/freestore-mgmt.html#faq-16.3

If you want to propound the virtues of Linux and C, start a thread in the Lounge. Don't hijack other people's threads!

Until then, I'm marking your posts as bad because they don't offer the OP any useful information.

[matty3269]

Thanks for the reply. In a response to the "Language wars" the function "popen" is a c function and i can not find an equivalent in C++. Therefore i had to use some c functions to read the pipe into C++ concepts.

[simo110]

To matty3269 :
I am so sorry for the appearance of the so sharp saying!
You can reference all the post! Choose one!
In my option, you just need to encapsulate the posix's c function "popen"!Choose your familiar language!

To Duoas :
For your option,I have my different option !But I just want to say nothing!

Because it is not polite to matty3296!

[weaknessforcats]

From Duoas:

Listen you idiots.

The OPs question has absolutely nothing to do with choice of language, except that he already knows he is mixing C into C++. Jumping up and down and telling him to forget C++ does not help; moreover, it is a waste of bandwith because nobody cares.

Yes, I comepletely understand your position. I have seen this many times. It seems dissent is not allowed. Therefore, to avoid any future conflicts of this nature, I have removed my article from the Articles forum and will no longer make any further posts to this site.

I extend apologies to matty3296.

[bnbertha]

I have read this thread with bewilderment. What the hell happened here? C and C++ have to be 'mixed' all the time; C is 32 key words, C++ is 64 including the 32 from C. Why would anyone say don't use C? You would lose half the C++ language. And what exactly has it got to do with the original question?

This is a storm in a tea cup, and as for throwing your teddy out the cot and leaving the site over it???

Anyone reading this thread and getting worried about using the likes of malloc() in c++ code, don't worry about it. new() is better but malloc() is fine.