3

I'm trying to traverse through my linked list and combine each of the nodes fields into a string, and then add the string into an array of strings.

void listToArray(){
   //create array of strings
    char *list[numNodes];
    int i = 0, n;

  while(head != NULL){
    // add string to array
    printf("[%d] %s %s:%d\n ", i, head->fileName, head->ip, head->port);
    n = sprintf(list[i], "[%d] %s %s:%d\n", i, head->fileName, head->ip, head->port);
    head = head->next;
    printf("%s\n", list[i]);
    i++;

 }

The printf statement works fine, which shows that it is not the fields of node that are causing the problem, but when I do my sprintf statement to copy the string into the index of the array. I get a segmentation fault.

Improve this question
5
  • 1
    you have to allocate memory for list[i] Commented Oct 7, 2015 at 22:07
  • When you declare, char *list[numNodes]; it allocates space for an array of char pointers, but they aren't initialized yet. You need to allocate them (e.g., using malloc for each element: for ( i = 0; i < numNodes; i++ ) list[i] = malloc(stringSize);). Commented Oct 7, 2015 at 22:07
  • Where do you allocate the memory for each not of list? list is defined to be a pointer to an array of characters but the porinter doesn't point to anything. You need to malloc or something. What I would do is make a character array called temp on the stack that is big enough (maybe 128 characters?) like char temp[128]; then sprintf into temp and then set list[i] = strdup(temp); Commented Oct 7, 2015 at 22:12
  • Instead of sprintf(), use asprintf() which will automatically allocate a memory large enough to hold the resulting string including the null byte terminator. However, this is a GNU extension, also available in *BSD, but not in POSIX. Commented Oct 7, 2015 at 22:41
  • I get a segmentation fault. -- Lucky you! Far worse things could have happened (e.g., appeared to "work" in a manner you thought was correct, only to fail fatally later). Commented Oct 7, 2015 at 22:44

3 Answers 3

Reset to default
4

You only declare 

char *list[numNodes];

but not allocate memory for them. Before using list[i] allocate memory with malloc. To know the size of generated string from sprintf use snprintf. Thanks user3121023 for finding us this function.

void listToArray(){
   //create array of strings
    char *list[numNodes];
    int i = 0, n;

  while(head != NULL){
    printf("[%d] %s %s:%d\n ", i, head->fileName, head->ip, head->port);

    n = snprintf(NULL, 0, "[%d] %s %s:%d\n", i, head->fileName, head->ip, head->port);
    list[i] = malloc((n+1)*sizeof(char));

    n = sprintf(list[i], "[%d] %s %s:%d\n", i, head->fileName, head->ip, head->port);
    head = head->next;
    printf("%s\n", list[i]);
    i++;

 }
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Quick note: If you use GCC, there's also asprintf which will do the allocation for you.
1

list is defined but each element has no/random pointer in it. The above answers are complete but this can be simplified a bit:

void listToArray(){
    //create array of strings
    char *list[numNodes];
    char buf[5000];
    int i = 0, n;

    while (head != NULL) {
        snprintf(buf,sizeof(buf),"[%d] %s %s:%d\n ",
            i,head->fileName,head->ip,head->port);

        // add string to array
        list[i] = strdup(buf);

        // output
        fputs(list[i],stdout);

        head = head->next;
        i++;
    }
}

You only need to do a single printf/sprintf. That's the slow part. Doing a single snprintf and strdup is faster and simpler than doing 2-3 *printf calls. I'm guessing that you only want one output line per item and the rest were for debug.

Improve this answer

Comments

0

You are not initializing the array of char pointers-- sprintf is writing data to some random location.

Each char pointer should be initialized to a buffer with malloc before you call sprintf.

Improve this answer

5 Comments

How can I do that without knowing the size of the string yet?
You can get the size of the formatted string first by using snprintf, then allocating, then using sprintf on the allocated string.
@owacoder - That will work using C99 standard as stated in man snprintf(3) - Concerning the return value of snprintf(), SUSv2 and C99 contradict each other: when snprintf() is called with size=0 then SUSv2 stipulates an unspecified return value less than 1, while C99 allows str to be NULL in this case, and gives the return value (as always) as the number of characters that would have been written in case the output string has been large enough. - Nothing wrong with your suggestion just putting this note out there for OP and future readers.
@alvits - So what's your suggestion for portability? Or is there a portable method?
@owacoder - I haven't found a portable method yet. But I do write a local implementation of asprintf() wrapped within compiler directive to use on non GNU systems. In short, this local asprintf() will do what GNU extension asprintf() would have done. I would love to hear from anyone who has found a portable method.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.