Getting input appended to linked list without getting the same input again - C

Question

I am trying to get inputs from the user and then append the struct to the end of the linked list. This works fine but I want to add another feature that would prevent the input being added if all the details are exactly the same. (In this case email, class, first and last name)

The for loop I added in the middle is what I tried to do to achieve this feature. The program goes into the loop without any problems but the input will still be added. How would I fix this?

struct request *append(struct request *list){

    char f_name[NAME_LEN+1];
    char l_name[NAME_LEN+1];
    char e_address[EMAIL_LEN+1];
    char c_name[CLASS_LEN+1];

    //get input
    printf("\nEnter email: ");
    scanf("%s", e_address);
    printf("\nEnter class: ");
    scanf("%s", c_name);
    printf("\nEnter child first name: ");
    scanf("%s", f_name);
    printf("\nEnter child last name: ");
    scanf("%s", l_name);

    //allocate memory for the structure
    struct request* p = (struct request*) malloc(sizeof(struct request)); 
    struct request *temp = list;

    //////////WHAT I TRIED BUT DIDN'T WORK
    for (p = list, temp = NULL; p != NULL; temp = p, p = p -> next) {
        if (strcmp(p -> first, f_name) == 0 && strcmp(p -> last, l_name) == 0 && strcmp(p -> email, e_address) == 0 && strcmp(p -> class, c_name) == 0) {
            printf("Output: request already exists");
            return list;
        }
    }

    //store the data
    strcpy(p->first, f_name);
    strcpy(p->last, l_name);
    strcpy(p->email, e_address);
    strcpy(p->class, c_name);
    p->next = NULL; 

    //if list is empty return pointer to the newly created linked list
    if (list == NULL) {
       return p; 
    }

    //traverse to the end of the list and append the new list to the original list
    while (temp->next != NULL) {
        temp = temp->next;
    }
    temp->next = p;


    return list;

}

Your loop immediately leaks the memory that you allocated just prior, because it reassigns the value stored in `p`. One important note on style... do not put spaces between your struct indirections: use `p->first` instead of `p -> first`. This will make your code more readable. Only complete beginners put spaces there, and usually someone has to tell them off about it before they stop doing it. — paddy, Nov 09 '20 at 01:55
You are mixing *"interface"* and *"implementation"*, don't do that. Your *"interface"* is the user-interface that prompts the user and receives and **validates** all user input. Your *"implementation"* is your data handling, your linked-list. Separate the two. Create a function that handles the interface with the user and fills a `struct request`. Then your implementation would be an `add()` or `push()` function that adds the `struct request` as a node in the list. Keeping interface and implementation separate will allow you to do a better job on both and make your code easier to maintain. — David C. Rankin, Nov 09 '20 at 05:54

Protofall · Answer 1 · 2020-11-09T03:52:02.123

First off, with those scanf's you should do something like this to avoid buffer overflows.

As to your question, you should malloc p after your for-loop since we should only allocate it if there is no other node with the exact same info (And as paddy said, in your code it causes a memory leak because you set p to point to something else hence losing that newly malloc-ed data).

Your loop is more complex than it needs to be. Admittedly I can't see why the for-loop wouldn't detect a copy, maybe providing some input cases would help clear it up? Either way I'd replace that section of the code with this:

// Get a pointer to the head of the list
struct request *temp = list;

// Find and node with the exact same data
while(temp->next != NULL) {
    if(!strcmp(temp->first, f_name) && !strcmp(temp->last, l_name) && !strcmp(temp->email, e_address) && !strcmp(temp->class, c_name)){
        printf("Output: request already exists");
        return list;
    }
    temp = temp->next;
}

// Allocate p and reset temp
struct request *p = (struct request*) malloc(sizeof(struct request)); 
temp = list;

// etc

Getting input appended to linked list without getting the same input again - C

1 Answers1