Adding to char array isn't working

Question

I'm trying to read a text file line by line, and add each line to a char array. But the lines aren't added, at all.

//This is the default char array that comes with the cURL code.
char *text[]={
  "one\n",
  "two\n",
  "three\n",
  " Hello, this is CURL email SMTP\n",
  NULL
};

/*Now we're going to replace that char array, with an array that holds the contents of a textfile. 
  We'll read a textfile out line by line, and add each line to the char array. 
*/
void makemailmessage()
{
    text[0] = '\0'; //Clear text
    text[0] = "testy\n"; //First line in new char array

    //Read the text file, add each line to the char array.
    string line;
    ifstream myfile ("C:\\Users\\admin\\Downloads\\bbb.txt");
    int counter;
    counter = 1;
    if (myfile.is_open())
    {
        while ( myfile.good() )
        {
          getline (myfile,line);

            //Convert the string variable "line" to a char (a)
            char *a=new char[line.size()+1];
            a[line.size()]=0;
            memcpy(a,line.c_str(),line.size());

            //Add \n to the end of "a" (new char will be "str")
            char str[80];
            strcpy (str,a);
            strcat (str,"\n");

            //Add "str" to the char array "text"
            text[counter] = str;
            text[counter+1] = "test\n"; //Also added this for testing purposes

            write_data("C:\\Users\\admin\\Downloads\\checkit.txt", str); //Also for testing purposes

        //Increase counter by 2 because we added two new items to the char array "text"
        counter++;
        counter++;
        }
    myfile.close();

    text[counter-1] = "testy2\n"; //Ad another text line
    text[counter] = NULL; //End char array
}

Each str is written correctly to checkit.txt but for some reason it is not added to the char array because I end up with the char array looking like this:

testy

test

test

testy2

What am I doing wrong?

UPDATE2: The reason I am trying to make a char array is because the cURL function I am using needs a char array to form the email body. This is the important part of the cURL code.

static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userp)
{
  struct WriteThis *pooh = (struct WriteThis *)userp;
  const char *data;

  if(size*nmemb < 1)
    return 0;

  data = text[pooh->counter]; //This part is using the char array.

  if(data) {
    size_t len = strlen(data);
    memcpy(ptr, data, len);
    pooh->counter++;
    return len;
  }
  return 0;
}

Here's the full code

Answer 1

Okay, after chatting on this a bit more, here is a fix:

C++ version

Full code file here: https://gist.github.com/1342118#file_test.cpp

Replace the relevant code with:

#include <vector>
#include <fstream>

// ...

std::vector<std::string> text;

static int read_text(char* fname)
{
    //Read the text file, add each line to the char array.
    std::ifstream myfile (fname);

    std::string line;
    while (std::getline(myfile, line))
        text.push_back(line + '\n');

    return 0;
}

static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userp)
{
    /* This was already in. */
  struct WriteThis *pooh = (struct WriteThis *)userp;

  if(size*nmemb < 1)
    return 0;

  if (pooh->counter < text.size())
  {
      const std::string& data = text[pooh->counter];

      memcpy(ptr, data.data(), data.length());
      pooh->counter++; /* advance pointer */
      return data.length();
  }
  return 0;                         /* no more data left to deliver */
}

Pure C version

Full code file here: https://gist.github.com/1342118#file_test.c

Replace

//This is the default char array that comes with the cURL code.
char *text[]={
  "one\n",
  "two\n",
  "three\n",
  " Hello, this is CURL email SMTP\n",
  NULL
};

With

char **text = 0;

static int read_text(char* fname)
{
    unsigned capacity = 10;
    int linecount = 0;

    // free_text(); see below
    text = realloc(text, capacity*sizeof(*text));

    FILE* file = fopen(fname, "r");
    if (!file)
        { perror("Opening file"); return 1; }

    char buf[2048];
    char* line = 0;

    while (line = fgets(buf, sizeof(buf), file))
    {
        if (linecount>=capacity)
        {
            capacity *= 2;
            text = realloc(text, capacity*sizeof(*text));
        }
        text[linecount++] = strdup(line);
    } 

    fclose(file);

    return 0;
}

Hook it up in you main function, e.g. like so

if (argc<2)
{
    printf("Usage: %s <email.eml>\n", argv[0]);
    exit(255);
} else
{
    printf("Reading email body from %s\n", argv[1]);
    if (0 != read_text(argv[1]))
        exit(254);
}

Or, if you so prefer, just calling read_text("C:\\Users\\admin\\Downloads\\bbb.txt") :)

To really top things off, don't forget to reclaim memory when you're done - properly:

#include "curl/curl.h" 

#include <stdio.h> 
#include <stdlib.h> 

#include <unistd.h> 
#include <memory.h>
#include <string.h>
#define GetCurrentDir getcwd 

#define USERNAME "obscured@gmail.com"
#define PASSWORD "obscured"
#define SMTPSERVER "smtp.gmail.com"
#define SMTPPORT ":587"
#define RECIPIENT "<obscured@gmail.com>"
#define MAILFROM "<obscured@gmail.com>"

#define MULTI_PERFORM_HANG_TIMEOUT 60 * 1000

/* Note that you should include the actual meta data headers here as well if
   you want the mail to have a Subject, another From:, show a To: or whatever
   you think your mail should feature! */
char **text = 0;

void free_text()
{
    if (text)
    {
        char** it;
        for (it = text; *it; ++it)
            free(*it);
        free(text);
        text = 0;
    }
}

static int read_text(char* fname)
{
    unsigned capacity = 10;
    int linecount = 0;

    free_text();
    text = realloc(text, capacity*sizeof(*text));

    FILE* file = fopen(fname, "r");
    if (!file)
        { perror("Opening file"); return 1; }

    char buf[2048];
    char* line = 0;

    while (line = fgets(buf, sizeof(buf), file))
    {
        if (linecount>=capacity)
        {
            capacity *= 2;
            text = realloc(text, capacity*sizeof(*text));
        }
        text[linecount++] = strdup(line);
    } 

    if (linecount>=capacity)
        text = realloc(text, (++capacity)*sizeof(*text));

    text[linecount] = 0; // terminate

    fclose(file);

    return 0;
}

struct WriteThis {
  int counter;
};

static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userp)
{
    /* This was already in. */
  struct WriteThis *pooh = (struct WriteThis *)userp;
  const char *data;

  if(size*nmemb < 1)
    return 0;

  data = text[pooh->counter];

  if(data) {
    size_t len = strlen(data);
    memcpy(ptr, data, len);
    pooh->counter++; /* advance pointer */
    return len;
  }
  return 0;                         /* no more data left to deliver */
}

static struct timeval tvnow(void)
{
  /*
  ** time() returns the value of time in seconds since the Epoch.
  */
  struct timeval now;
  now.tv_sec = (long)time(NULL);
  now.tv_usec = 0;
  return now;
}

static long tvdiff(struct timeval newer, struct timeval older)
{
  return (newer.tv_sec-older.tv_sec)*1000+
    (newer.tv_usec-older.tv_usec)/1000;
}

int main(int argc, char** argv)
{
    if (argc<2)
    {
        printf("Usage: %s <email.eml>\n", argv[0]);
        exit(255);
    } else
    {
        printf("Reading email body from %s\n", argv[1]);
        if (0 != read_text(argv[1]))
            exit(254);
    }

   CURL *curl;
   CURLM *mcurl;
   int still_running = 1;
   struct timeval mp_start;
   char mp_timedout = 0;
   struct WriteThis pooh;
   struct curl_slist* rcpt_list = NULL;

   pooh.counter = 0;

   curl_global_init(CURL_GLOBAL_DEFAULT);

   curl = curl_easy_init();
   if(!curl)
     return 1;

   mcurl = curl_multi_init();
   if(!mcurl)
     return 2;

   rcpt_list = curl_slist_append(rcpt_list, RECIPIENT);
   /* more addresses can be added here
      rcpt_list = curl_slist_append(rcpt_list, "<others@example.com>");
   */

   curl_easy_setopt(curl, CURLOPT_URL, "smtp://" SMTPSERVER SMTPPORT);
   curl_easy_setopt(curl, CURLOPT_USERNAME, USERNAME);
   curl_easy_setopt(curl, CURLOPT_PASSWORD, PASSWORD);
   curl_easy_setopt(curl, CURLOPT_READFUNCTION, read_callback);
   curl_easy_setopt(curl, CURLOPT_MAIL_FROM, MAILFROM);
   curl_easy_setopt(curl, CURLOPT_MAIL_RCPT, rcpt_list);
   curl_easy_setopt(curl, CURLOPT_USE_SSL, CURLUSESSL_ALL);
   curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER,0);
   curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);
   curl_easy_setopt(curl, CURLOPT_READDATA, &pooh);
   curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
   curl_easy_setopt(curl, CURLOPT_SSLVERSION, 0);
   curl_easy_setopt(curl, CURLOPT_SSL_SESSIONID_CACHE, 0);
   curl_multi_add_handle(mcurl, curl);

   mp_timedout = 0;
   mp_start = tvnow();

  /* we start some action by calling perform right away */
  curl_multi_perform(mcurl, &still_running);

  while(still_running) {
    struct timeval timeout;
    int rc; /* select() return code */

    fd_set fdread;
    fd_set fdwrite;
    fd_set fdexcep;
    int maxfd = -1;

    long curl_timeo = -1;

    FD_ZERO(&fdread);
    FD_ZERO(&fdwrite);
    FD_ZERO(&fdexcep);

    /* set a suitable timeout to play around with */
    timeout.tv_sec = 1;
    timeout.tv_usec = 0;

    curl_multi_timeout(mcurl, &curl_timeo);
    if(curl_timeo >= 0) {
      timeout.tv_sec = curl_timeo / 1000;
      if(timeout.tv_sec > 1)
        timeout.tv_sec = 1;
      else
        timeout.tv_usec = (curl_timeo % 1000) * 1000;
    }

    /* get file descriptors from the transfers */
    curl_multi_fdset(mcurl, &fdread, &fdwrite, &fdexcep, &maxfd);

    /* In a real-world program you OF COURSE check the return code of the
       function calls.  On success, the value of maxfd is guaranteed to be
       greater or equal than -1.  We call select(maxfd + 1, ...), specially in
       case of (maxfd == -1), we call select(0, ...), which is basically equal
       to sleep. */

    //rc = select(maxfd+1, &fdread, &fdwrite, &fdexcep, &timeout);

    if (tvdiff(tvnow(), mp_start) > MULTI_PERFORM_HANG_TIMEOUT) {
      fprintf(stderr, "ABORTING TEST, since it seems "
              "that it would have run forever.\n");
      break;
    }

    switch(rc) {
    case -1:
      /* select error */
      break;
    case 0: /* timeout */
    default: /* action */
      curl_multi_perform(mcurl, &still_running);
      break;
    }
  }

  curl_slist_free_all(rcpt_list);
  curl_multi_remove_handle(mcurl, curl);
  curl_multi_cleanup(mcurl);
  curl_easy_cleanup(curl);
  curl_global_cleanup();
  free_text();
  return 0;
}

Answer 2

I'm trying to read a text file line by line, and add each line to a char array.

Since this is C++, why not use an std::vector<string> and use the std::string version of getline?

The std::string class will look after the memory needed to hold a string of any sort of length, and the std::vector class will worry about the memory needed to hold an "array", so to speak, of strings.

EDIT: Actually looking at your code again, you do use an std::string and then allocate memory to store it as an array of chars, and then store pointers to those strings in some fixed sized array, test. Why go to all that trouble when, as I mentioned above, you can use an std::vector<string> to hold all your std::string objects? Mind = boggled.

EDIT2: Couldn't you also use cURLpp as a C++ wrapper for cURL? I haven't used either so I can't comment on the effectiveness of it.

Answer 3

What am I doing wrong?

For one, this:

    char str[80];
    strcpy (str,a);
    strcat (str,"\n");

    //Add "str" to the char array "text"
    text[counter] = str;

str is allocated on the stack, with block-wide scope. Then you enter that pointer in an array with a greater scope. This is usually a recipe for disaster - a rather impressive segmentation fault or whatever the equivalent is on your platform.

In this case, due to its use in a loop, your program will either crash, or - if the stars have the proper alignment - you will end up with all pointers in your array pointing to the same out-of-scope string, namely the one that was last read.

Why do you even go into that trouble, when you have already dynamically allocated a in the heap?

By the way, mixing char[] arrays (and the associated standard C library functions) with C++ strings is NOT a good idea. Not even an acceptable one. OK, it a bad idea. Just stick to C++ strings...