1

For this assignment I'm supposed to grab multiple names from the command line arguments which are prefaced with either '+' (which inserts the name into my linked list) or '-' (which removes the name from the list), so if I were to enter "+bill +jim +ted -bill", I would add those three names without the '+' and remove bill. Ive tried to dynamically allocate a string using malloc so I can modify the name string, but my code doesnt insert the string data into my linked list, and I get a free(): invalid pointer error when I pass through some test strings. If I move the free(name) statement to the if/else if statements I get a segmentation fault. How would I go about dynamically allocating these strings from the command line correctly so they can be modified?

int main(int argc, char *argv[]){
    struct node* head;
    for(int x = 0; x < argc; x++){
            char * name = malloc((strlen(argv[x])+1));
            name = argv[x];
            if(name[0] == '+'){
                    printf("adding %s \n", name++);
                    insert(&head, name++);
                    printf("List: ");
                    printList(&head);
            }
            else if(name[0] == '-'){
                    printf("removing %s \n", name++);
                    removeNode(&head, name++);
            }
            free(name);
    }
}

char * removeNode(struct node** head, char* name){
        struct node *current = *head;
        struct node *previous;

        if(current == NULL){
                return "error0";
        }

        if(current->data == name){
                struct node * node2delete= *head;
                *head = node2delete->next;
                char * name2 = malloc(strlen(name)+1);
                strcpy(name2, name);
                free(node2delete);
                printf("Removed %s \n", name);
                return name2;
        }
        while(current != NULL){
                previous  = current;
                current = current->next;
                if(current->data == name){
                        previous->next = current->next;
                        char * name2 = malloc(strlen(name)+1);
                        strcpy(name2, name);
                        free(current);
                        printf("Removed %s \n", name);
                        return name2;
                }
        }
        return "error0";
}

2 Answers2

2

Try using strcpy(name, argv[x]); instead of name = argv[x], because when you assign a pointer to a pointer, they will lead to the same memory location where your null-terminated char array is stored. So that you could've done a memory deallocation for the argv[x] when you called free(name);

The pointer opperations are not the same as value assignments.

also see this thread: How do malloc() and free() work?

Ivan Silkin
  • 381
  • 1
  • 7
  • 15
0

Many problems

Incorrect copying of string

    //char * name = malloc((strlen(argv[x])+1));
    //name = argv[x];
    char * name = malloc(strlen(argv[x])+1);
    if (name == NULL) return EXIT_FAILURE; // Out of memory
    strcpy(name, argv[x]);

Incorrect free()

Code needs to free the same value received from allocation. name++ messes that up.

Uninitialized head*

// struct node* head;
struct node* head == NULL;

More

Likely only need to allocate when adding and allocated after the +/-.

int main(int argc, char *argv[]){
  struct node* head = NULL;
  // for(int x = 0; x < argc; x++){
  for(int x = 1; x < argc; x++){  // Skip adding the program name, start at 1
    if(argv[x][0] == '+'){
      char * name = malloc((strlen(argv[x] + 1)+1));
      if (name == NULL) return EXIT_FAILURE; // Out of memory
      strcpy(name, argv[x]);
      printf("adding %s \n", name);  // No ++
      insert(&head, name); // No ++
      printf("List: ");
      printList(&head);
    }
    else if(argv[x][0] == '-'){
      printf("removing %s \n", argv[x] + 1);
      // This is a problem, I'd expect removeNode() to return a pointer to the free'd name
      char *name = removeNode(&head, argv[x] + 1);
      free(name);
    }
  }
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • How would I return a pointer to freed memory? Would I need to use malloc and strcpy to return a pointer to the node I deleted? – Torin Costales Nov 30 '21 at 19:51
  • @TorinCostales Do _not_ return a pointer to free'd memory. Instead, have `char *removeNode()` return a pointer that the calling code will later free. `removeNode()`can delete the _node_, but should not delete the string pointer it saved. Much depends on the details of `removeNode()`, which since is not shown, can only be superficially discussed. – chux - Reinstate Monica Nov 30 '21 at 21:05
  • So I was working on this function and it is now shown. Ive tried to dynamically allocate another string which will be returned, however it still results in a segmentation error. I tried to just return the parameter {name}, but that was also another segmentation error. What is the correct way to go about returning a pointer? Im new to dynamic memory and it kinda makes my head spin – Torin Costales Nov 30 '21 at 21:14
  • I'd expect something like `if(strcmp(current->data, name) == 0){ char *data = current->data; previous->next = current->next; free(current); printf("Removed <%s> \n", data); return data; }`. Yet that depends on the unseen `insert()`. Sorry no time for more. Good luck. – chux - Reinstate Monica Nov 30 '21 at 21:24