strcpy() to copy string from struct member to char array is failing. Why?

Question

I have a structure node which is used to create a binary search tree. Inside each node I am storing a integer KEY and a corresponding string value. I am performing some searches within the tree and wish to return arrays containing key value pairs of only specific nodes.

TO do so I am passing arrays by reference and saving the integer KEY to that array. This works fine, however when I try to the the same with the string I am getting poor results.

In the below code I am trying to copy the string inside root[count].value; to p_value_arr[*p_unique_count] which is a char array.

Struct definition:

   typedef struct node {
        int KEY;
        char *value;
        int node_count;
        struct node *left, *right;
        int unique_count;
    } node;

Function to traverse graph and copy unique key value pairs. KEY is being copied correctly to an array while value is not.

void unique_key(node *root, int *p_unique_count, int p_unique_arr[], char *p_value_arr[]) { 
    int count = 0;      
    //unique *temp = (unique *)malloc(n * sizeof(unique));
        
    if (root != NULL)
    {
        unique_key(root->left, p_unique_count, p_unique_arr, p_value_arr);
        if (root->node_count == 1) {

            root[count].unique_count = *p_unique_count;
            p_unique_arr[*p_unique_count] = root[count].KEY;
            printf("%s\n", root[count].value);
            //"warning: assignment makes integer from pointer without a cast"
            strcpy(p_value_arr[*p_unique_count],root[count].value);  
                        
            printf("%d(%d) -> %s %d\n", root->KEY, root->node_count, root->value, root->unique_count);
                        (*p_unique_count)++;
            count++;
        }
        unique_key(root->right, p_unique_count, p_unique_arr, p_value_arr);
    }
}

A utility function to insert a new node with given key in BST

node* insert_node(node* node, int key, char *value)
{
    /* If the tree is empty, return a new node */
    if (node == NULL) 
        return newNode(key,value);

    // If key already exists in BST, icnrement count and return 
    if (key == node->KEY)
    {
        (node->node_count)++;
    //  return node;
    }

    /* Otherwise, recur down the tree */
    if (key < node->KEY)
        node->left = insert_node(node->left, key, value);
    else
        node->right = insert_node(node->right, key, value);

    /* return the (unchanged) node pointer */
    return node;
}

node *newNode(int KEY, char *value)
{
    struct node *temp = (struct node *)malloc(sizeof(struct node));
    temp->KEY = KEY;
    strcpy(temp->value, value);
    temp->left = temp->right = NULL;
    temp->node_count = 1;
    return temp;
}

Main driver code

int main() {
    int unique_count = 0;
    int in_count = 0;
    int unique_arr[10]; /
    char *value_arr[10];   // an array of pointers  
    
    /* Let us create following BST.  Passing values along with key */
    node *root = NULL;
    
    //this is for storing commands 
    root = insert_node(root, 2, "Hello");
    root = insert_node(root, 3, "Thanks");

    printf("\nkeys of the given tree \n");
    unique_key(root, &unique_count, unique_arr, *value_arr);
    for(int i = 0; i < 10; i++) {                
        printf("%d %s\n", unique_arr[i], value_arr[i]);  //Mismatching the argument type "char" and conversion specifier "s" and nothing prints here
    }
   
}

Output:

Hello

keys of the given tree

Segmentation fault

Any suggestions on how I can effectively copy a string inside a struct member to an array of chars?

EDIT:

Full code: https://pastebin.com/CB4Gp0gY

Since char *value_arr[10]; is an array of pointers I followed chapter 5.6 of K&R The C programming language to pass the array of pointers to the function. I get no warnings now, but the seg fault persists.

I also have more warnings set on my NetBeans 8.2.

Output from debugger:

/cygdrive/C/Users/****/AppData/Roaming/NetBeans/8.2/bin/nativeexecution/dorun.sh: line 71: 16516 Segmentation fault      (core dumped) sh "${SHFILE}"

You are assigning values of type char * to array elements of type char. If your compiler is not emitting warnings about that, then turn up the warning level. In any case, that appears to be your problem. — John Bollinger
– John Bollinger, Commented Dec 7, 2018 at 15:17
temp->value = value; this is a soft copy of the pointer, not a copy of the string itself. Meaning all your value pointers will potentially point at the same text. See How to correctly assign a new string value? If you are coming from some higher-level language, you need to acknowledge that C does not have a string class. — Lundin
– Lundin, Commented Dec 7, 2018 at 15:19
The compiler is your friend. Ensure that the warning levels are turned up, and pay attention to the warnings it issues. In particular, pay attention to (and resolve) the warnings you should now be getting about mismatched pointer types in the call to function unique_key. — John Bollinger
– John Bollinger, Commented Dec 7, 2018 at 16:59
Why is this not a specific programming question? I am unable to use strcpy() to copy strings from a struct member to an array. Please advise and I will try to make the question more specific. — rrz0
– rrz0, Commented Dec 7, 2018 at 17:31
I don't see where you actually allocate any space for the char arrays. I see you allocating space for a node, and then immediately copying a string to an unitialized (and potentially completely random) pointer. See @Lundin's link. — Phil M
– Phil M, Commented Dec 7, 2018 at 18:10

Michael Dorgan · Accepted Answer · 2018-12-07 18:21:26Z

0

Gonna follow up for Lundin here

node *newNode(int KEY, char *value)
{
  // Allocate a new overall structure
  struct node *temp = (struct node *)malloc(sizeof(struct node));

  //Copy the integer key
  temp->KEY = KEY;

  // uh oh - copy the given string into a random location in memory and segfault.
  // Hint - you need to allocate enough memory to hold the incoming string.
  // Advanced hint - If you don't want to make a copy of the string, you can 
  // just store its pointer, but it would want to be marked constant at the least... 
  strcpy(temp->value, value);

  // Set tree stuff and count, but we are already dead...
  temp->left = temp->right = NULL;
  temp->node_count = 1;
  return temp;
}

Also,

printf("%d %s\n", unique_arr[i], value_arr[i]);  //Mismatching the argument type "char" and conversion specifier "s" and nothing prints here

Will fail because value_arr[i] is not a string, it is a char *. For this to work, it would have to point at a valid C string, or would need to point to memory that has a properly '\0' terminated string.

Take a look at his given link as you need a deeper understanding of how C strings work.

answered Dec 7, 2018 at 18:21

Michael Dorgan

12.5k3 gold badges36 silver badges63 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

rrz0 Over a year ago

Thanks for your answer. I am printing root[count].value correctly, after I store using temp->value = value;. Wouldn't this indicate that *newNode is storing the string correctly?

Michael Dorgan Over a year ago

As-is, you are invoking undefined behavior (UB) with your strcpy(). As such, anything can happen. Since you are having issues with string memory management, consider using fixed size char arrays until you get the hang of them.

rrz0 · Accepted Answer · 2018-12-08 07:40:11Z

0

char *value_arr[10]; // the problem is here

That initializes an array of pointers but does not assign memory to those pointers before using them for stuff like strcpy(). As per K&R chapter 5.6 one should allocate memory using alloc() for the pointer array inside the function.

If you want a pointer to point to some memory for storing a string, then you have to create such an area of memory and set the pointer to point to it.

 char *p;

 alloc(strlen(root[count].value) +1);

 strcpy(p, root[count].value);
 p_value_arr[*p_unique_count] = p;

edited Dec 8, 2018 at 7:40

answered Dec 8, 2018 at 7:23

rrz0

2,3228 gold badges33 silver badges68 bronze badges

Comments

eljok7 · Accepted Answer · 2022-11-12 22:38:40Z

struct node_t*curr = head;

  struct node_t*node = (struct node_t*)malloc(sizeof(struct node_t));
  strcpy(node -> str,str);
  node -> prev = NULL;
  node -> next = NULL;

  if(curr == NULL)
  {
      head = node;
      tail = node;

      return
  }

  int value = strcmp(curr -> str,str);

  if(value>0)
  {
    head = node;
    node -> next = curr;
    curr -> prev = node;
    return;
  }

  struct node_t* prev = curr;
  curr = prev -> next;

  while(curr != NULL)
  {
    value=strcmp(prev -> str,str);
    if(value < 0)
    {
      int value1 = strcmp(curr -> str,str)

      if(value1>0)
      {
        node -> prev = prev;
        node -> next = curr;
        node -> next = node;
        node -> prev = node;
      }
      else if(value1 == 0)
      {
        if(curr -> next == NULL)
          tail=prev;

        prev -> next = curr -> next;
        curr -> prev = NULL
        return;

      }
    }
    prev = curr;
    curr = prev -> next;
  }
  prev -> next = node;
  node -> prev = prev;
  tail = node;

Collectives™ on Stack Overflow

strcpy() to copy string from struct member to char array is failing. Why?

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related