-1

I am having an issue with copying the contents of the character array in a linked list to a regular array of characters. I have an issue with a segmentation fault that I am not sure why.

The program that I have created works when the character array in the linked list is only one character, but it does not work when it is greater than 1. The main issue occurs on line 62 ("array[index] = p -> word[count]"). I have tried using strcpy to copy each index of it into the character array but that as well produced an error that reads: "passing argument 2 of ‘strcpy’ makes pointer from integer without a cast". However, when I use an assignment statement, I just get a segmentation fault. I am not sure why because I feel I've created enough memory that should be able to hold the contents of the linked list for the array.

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

typedef struct node
{
    char word[100];
    struct node *next;
} ListNode;

int main ()
{
    ListNode * head = NULL;
    ListNode * tail = NULL;

    for (int count = 0; count < 5; count++)
    {
        ListNode * temp = malloc (sizeof (*temp));

        strcpy(temp -> word, "Hi");
        temp -> next = NULL;

        if (tail == NULL)
        {
            head = temp;
            tail = temp;
        }
        else
        {
            tail->next = temp;
            tail = temp;
        }
    }

    char array[999]; // array that will hold the characters in the linked list

    ListNode * p = head; //position of the current node
    int count;
    int index = 0;


    // while p is still a node in the list
    while(p != NULL)
    {
        if((int) strlen(p -> word) > 1) // checks if the string is longer than one character
        {
          count = 0; // initializes count as 0

          while(count < (int) strlen(p -> word)) // counts how many characters are in the string
          {
            array[index] = p -> word[count]; // assings the words into charater array
            count++; // increments the count
            index++; // changes the index
          }
        }
        else
        {
          array[index] = p -> word[0]; // copies p-word to array
          index++; // changes the index in the array
          p = p -> next;
       }
    }

    return 0;
}

As mentioned before, the program works whenever the character array in the linked list is only 1, but a segmentation fault is produced when the number is greater than 1. Please let me know what I need to correct in this program. Thank you.

Steve Friedl
  • 3,929
  • 1
  • 23
  • 30
africanxmamba
  • 91
  • 3
  • 13
  • `p = p -> next;` is conditional. -->> Intead, you could use a for-loop to avoid this kind of logic errors. – wildplasser Nov 03 '19 at 17:07
  • `ListNode * temp = malloc (sizeof (*temp));`. What does this statement mean? You are creating a memory of size (*temp) but what is temp? – Nikhil Badyal Nov 03 '19 at 17:09
  • 1
    Creates space to hold the contents of a ListNode @problematicDude – africanxmamba Nov 03 '19 at 17:12
  • How compiler will know the sizeof *temp ? Compiler don't know what is temp. – Nikhil Badyal Nov 03 '19 at 17:13
  • https://en.cppreference.com/w/c/memory/malloc read this about how malloc works. – Nikhil Badyal Nov 03 '19 at 17:14
  • 1
    That isn't the issue. And temp has already been declared as a ListNode. Therefore, it does know what temp is. ListNode * temp creates a pointer. When using malloc(sizeof(*temp)) I create space to hold the character array and the pointer for the following node. – africanxmamba Nov 03 '19 at 17:16
  • @AfricanMamba It's better to write `ListNode * temp = malloc (sizeof(ListNode));` I can't remember off the top of my head, but the compiler hasn't allocated any space for `*temp` _until_ the malloc is complete. So it seems like you're attempting to allocate space the size of something that doesn't exist. – AdamInTheOculus Nov 03 '19 at 17:52
  • @AdamInTheOculus No, it is not *better*. It is *worse*, because it could be a source of errors. `THING *ptr = malloc (sizeof *ptr);` is always correct. `THING *ptr = malloc(sizeof(THIMG));` is an error waiting to happen.. – wildplasser Nov 03 '19 at 19:36
  • @wildplasser Care to elaborate more than just *it could be a source of errors*? `ListNode` was already defined as an alias for `struct node`. So looking at OP's struct, `sizeof(ListNode)` would return 108 bytes. – AdamInTheOculus Nov 04 '19 at 16:26
  • https://stackoverflow.com/a/15527407/905902 , for example. The general idea is: *Dont Repeat Yourself* – wildplasser Nov 06 '19 at 11:36

1 Answers1

0
  • simplify your loops; for-loops allow you to keep the loop-machinery on one line
  • avoid special cases; there is nothing special about a one-char string

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

typedef struct node
{
  char word[100];
  struct node *next;
} ListNode;

int main ()
{
  ListNode * head = NULL;
  ListNode * tail = NULL;
  ListNode * p ;
  int count;
  int index ;
  char array[999]; // array that will hold the characters in the linked list

  for (count = 0; count < 5; count++)
  {
    ListNode * temp = malloc (sizeof *temp);

    strcpy(temp->word, "Hi");
    temp->next = NULL;

    if (!tail) { head = temp; tail = temp; }
    else { tail->next = temp; tail = temp; }
  }

  count=0;
  for(p=head;p; p=p->next) { // walk the linked list

      for(index=0;  p->word[index]; index++) { // walk the string
        array[count++] = p->word[index];
      }

    }
  array[count++] = 0; // terminate

  printf("%s\n", array);
  return 0;
}
wildplasser
  • 43,142
  • 8
  • 66
  • 109