free strdup output in a loop

Question

I have a loop which tries to read data from a qml list object, here is my loop

char * argvarry[(gcps.size() * 5) + 8];
argvarry[0] = "-of";
argvarry[1] = "GTiff";
argvarry[2] = "-a_nodata";
argvarry[3] = "'0 0 0'";
argvarry[4] = "-a_srs";
argvarry[5] = a_srs;

int argc = 6;

for (int i = 0;i < gcps.size(); i++) {
    argvarry[argc] = "-gcp";argc++;//gcp_values
    gcppoint_ *a = qobject_cast<gcppoint_ *>(gcps.at(i).value<QObject *>());

      argvarry[argc]= strdup( const_cast<char*>(QString::number(a->row()).toStdString().c_str())); argc++;
      argvarry[argc] = strdup(const_cast<char*>(QString::number(a->column()).toStdString().c_str())); argc++;
      argvarry[argc] = strdup(const_cast<char*>(QString::number(a->lon()).toStdString().c_str())); argc++;
      argvarry[argc] =strdup( const_cast<char*>(QString::number(a->lat()).toStdString().c_str())); argc++;
}

well I tried some methods to make the above code work but none worked, for example If I use strdup it will work but it needs to be freed, I tried to save output of strdup in a variable and then free it in loop, but it freed all variables. here is my try

for(int i=0;i<vl.size();i++) {
    gcppoint_ *a = qobject_cast<gcppoint_ *>(vl.at(i).value<QObject *>());

      char* srcX =strdup(const_cast<char*>(QString::number(a->row()).toStdString().c_str()));
      char* srcY = strdup(const_cast<char*>(QString::number(a->column()).toStdString().c_str()));
      char* dstX =strdup( const_cast<char*>(QString::number(a->lon()).toStdString().c_str()));
      char* dstY = strdup(const_cast<char*>(QString::number(a->lat()).toStdString().c_str()));
      qDebug() <<srcX<<" " <<srcY << " " <<dstX<< " " <<dstY;
     argvarry[i]=srcX;

      if(srcX)
          free(srcX) //does not work it frees argvarry[i] too

}
//...do some thing with argvarry
free????

I noticed that all argvarry[i] will be freed too, so I can not use them anymore. well how can I make it work, I have too free strdup but I can not handle it. how can I change this loop and when I used argvarry I free strup result?

what If I change the above code to sth similar

char * srcX;
char * srcY;
char * dstX;
char * dstY;

for (int i = 0;i < gcps.size(); i++) {
    argvarry[argc] = "-gcp";argc++;//gcp_values
    gcppoint_ *a = qobject_cast<gcppoint_ *>(gcps.at(i).value<QObject *>());

      srcX= strdup( const_cast<char*>(QString::number(a->row()).toStdString().c_str())); 
      srcY = strdup(const_cast<char*>(QString::number(a->column()).toStdString().c_str())); 
      dstX = strdup(const_cast<char*>(QString::number(a->lon()).toStdString().c_str())); 
      dstY =strdup( const_cast<char*>(QString::number(a->lat()).toStdString().c_str()));
      argvarry[argc]=srcX;argc++;
      argvarry[argc]=srcY;argc++;
      argvarry[argc]=dstX;argc++;
      argvarry[argc]=dstY;argc++;
      }
          ///do things

         //free(srcX);free(srcY)

As you are using C++, not C, you shouldn't need functions like `strdup()` and `free()`. Just use a `std::vector` (or a vector of `QString`, or a `QStringList`, or whatever Qt recommends). — Karsten Koop, Nov 30 '17 at 16:17
@KarstenKoop hi, but I need to fill char * argvarry[(gcps.size() * 5) + 8]; and most I need to convert Qstring and std:string into char * and again I'll face making them free and a such problem — Majid Hojati, Nov 30 '17 at 16:36
@MajidHojati -- I am sure this can be accomplished much better without the `strdup` calls. — PaulMcKenzie, Nov 30 '17 at 17:14

Jeremy Friesner · Accepted Answer · 2017-11-30T20:19:49.050

1

  char* srcX = const_cast<char*>(QString::number(a->row()).toStdString().c_str());

The problem with the above is that toStdString() returns a temporary QString object, and since the QString object is temporary, it gets destroyed at the end of the line. That means that srcX is a dangling pointer when you try to use it later.

If you absolutely need an array of char * pointers, you'll need to make sure the data they point to remains valid for the lifetime of the array. strdup() is one way to do that, but as you noted, it has its own difficulties, in particular you'll need to manually call free() on the strings after you're done with them, or you'll leak memory.

Another approach would be to build up a list of std::string objects first, like this:

// Build up a list of std::strings
std::vector<std::string> strsList;
strsList.push_back("-of");
strsList.push_back("GTiff");
strsList.push_back("-a_nodata");
strsList.push_back("'0 0 0'");
strsList.push_back("-a_srs");
strsList.push_back("a_srs");
for (int i = 0;i < gcps.size(); i++) {
   strsList.push_back("-gcp");
   strsList.push_back(QString::number(a->row()).toStdString());
   strsList.push_back(QString::number(a->column()).toStdString());
   strsList.push_back(QString::number(a->lon()).toStdString());
   strsList.push_back(QString::number(a->lat()).toStdString());
}

// Now create an array of pointers to the data in those std::strings
// This array will remain valid for as long as strsList remains valid and unmodified
char * argvarray[strsList.size()];
for (size_t i=0; i<strsList.size(); i++) argvarray[i] = const_cast<char *>(strsList[i].c_str());

edited Nov 30 '17 at 20:19

answered Nov 30 '17 at 16:28

Jeremy Friesner

70,199
15
131
234

Hi, Thanks for your help, about first part I am using argc++ at the end of each line. So it increases. the argvarry have to be char * and I had a deficulty to convert std::string and Qstring info char * for example some functions converts them into const char * and it does not work, can you please guide me about freeing strup output after using them? – Majid Hojati Nov 30 '17 at 16:34
Can you please check my question?I added a new method, do you think it is safe? – Majid Hojati Nov 30 '17 at 16:41
Your new method looks safe (as long as we're willing to assume that strdup() will never return NULL), but it is likely to leak memory if you're not very careful to free() all of the strings you strdup()'d. (Note that you'll also need to be careful to *not* call free() on any string that you didn't strdup(), and since you're storing pointers to strdup()'d and statically-allocated strings all together in the same char array, the code is likely to be very fragile and error-prone) – Jeremy Friesner Nov 30 '17 at 16:44
Thanks for your help, I am trying your proposed method I'll share the results, thanks – Majid Hojati Nov 30 '17 at 16:47
"That means that srcX is a dangling pointer": `srcX` is the return value of `strdup`, not the temporary returned from the QString. – Karsten Koop Nov 30 '17 at 16:48
I tried to use your method but String::number is not familier to me, is it std::string? – Majid Hojati Nov 30 '17 at 16:53
Oops, that was supposed to be QString::number, I think autocorrect bit me somehow :/ – Jeremy Friesner Nov 30 '17 at 20:20
@KarstenKoop oops, I had meant to copy in the implementation that didn't have a call to strdup() in it. Fixed, thanks. – Jeremy Friesner Nov 30 '17 at 20:21

free strdup output in a loop

1 Answers1