Maintain a list of contacts in C

Question

I have a function that gets a string and prints it as a message. Then, it reads the text from stdin, saves it in a pointer and sends it back:

char *getString(char *queryText){
char buffer[BUFFERSIZE];
char *temp;
printf("%s", queryText);

fgets(buffer, BUFFERSIZE, stdin);

//clears \n at the end of the line
if ((strlen(buffer) > 0) && (buffer[strlen(buffer) - 1] == '\n'))
    buffer[strlen(buffer) - 1] = '\0';

temp = (char*)malloc(strlen(buffer));
//if(temp == NULL)? , checked each time in the calling function
strcpy(temp, buffer);
return temp;}

I use it every time I need to get an input from the user that needs to be saved dynamically. For example, in this function that adds a name to the char* contacts (that's just a part of it):

int add(char *contacts[], int *nextAvailableP){
if (*nextAvailableP > N-3){
    printf("Unable to add a contact, not enough memory\n");
    return FALSE;
}

int numberOfMails = 0;

//receives the name, and saves it at the right cell
contacts[*nextAvailableP] = getString("Enter the contact name\n");
if(contacts[*nextAvailableP] == NULL){
    printf("Malloc failed\n");
    return FALSE;
}
(*nextAvailableP)++;

My questions are:

1. Is this really doing what i think it does?
2. Do I need to check if malloc succeeded in char *getString(char *queryText) or the way it is right now?
3. I'm just changing the pointer to the memory I've allocated, right? So I don't have to free it until I'm deleting the name from the array.
4. Is it a good habit at all?

Whole Program

For reference, here is the whole program.

My assigment was to write a program that maintains a list of contacts and their mail addresses. I had to use char *contacts[N]; and a variable that saves the index of the next available cell in the array.

For each contact the cell at index:

• i will save his name
• i+1 will save the number of mails (not known in advance)
• i+2 and beyond the mail addresses (not any format required)

I also had to

#define TRUE   1
#define FALSE  0

The program will present the user the following menu:

Choose an option:

1. Enter new contact (Had to check that there is enough room for the minimal contact)
2. Search contact
3. Delete mail from contact (only if he has more than 1 email adress)
4. Delete Contact (Delete everythings about him, and rearrange the array)
5. Print
6. Exit

We can assume that the maximum length of a name or a mail will be 50, and that the inputs are valid.

#include <stdio.h>
#include <stdlib.h>
#define TRUE   1
#define FALSE  0

const int N = 100;
const int BUFFERSIZE = 50;

void printMenu();
int add(char* contacts[], int* nextAvailableP);
char *getString(char *queryText);
void deleteMailAddress(char* contacts[], int* nextAvailableP);
int existReturnIndex(char* contacts[], int* nextAvailableP);
void deleteContact(char* contacts[], int* nextAvailableP);
void printArray(char* contacts[], int* nextAvailable);
void freeMemory(char* contacts[], int* nextAvailable);
int main()
{
    char *contacts[N];
    int nextAvailable = 0;
    int quit = FALSE;
    int option;
    printMenu();
    while(quit == FALSE){
        scanf(" %d", &option);
        getchar();
        switch(option){
        case 1:
            add(contacts, &nextAvailable);
            break;
        case 2:
            find(contacts, &nextAvailable);
            break;
        case 3:
            deleteMailAddress(contacts, &nextAvailable);
            break;
        case 4:
            deleteContact(contacts, &nextAvailable);
            break;
        case 5:
            printArray(contacts, &nextAvailable);
            break;
        case 6:
            freeMemory(contacts, &nextAvailable);
            quit = TRUE;
            break;

        default:
            printf("Enter a valid number between 1 to 6\n");
        }
        printMenu();
    }
}

//Prints the menu
void printMenu(){
    printf("Choose an option \n1. Enter new contact.\n2. Search contact.\n3. Delete mail from contact.\n4. Delete Contact.\n5. Print.\n6. Exit.\n\n");
}

void freeMemory(char *contacts[], int* nextAvailableP){
    int i;
    for(i=0; i<*nextAvailableP; i++){
        free(contacts[i]);
    }
    *nextAvailableP = 0;
}
void printArray(char *contacts[], int* nextAvailableP){
    int i;
    for(i=0; i<*nextAvailableP;){
        printf("%s:\n", contacts[i++]);
        int tempNumOfMails = atoi(contacts[i]);
        printXmailsNumberd(contacts, tempNumOfMails, ++i);
        i+=tempNumOfMails;
    }
}
//Gets a contact name and deletes it from the list if he exists, rearranging the array afterwards
void deleteContact(char *contacts[], int *nextAvailableP){
    int i = existReturnIndex(contacts, nextAvailableP);
    if(i != -1){
        int numberOfMails = atoi(contacts[i+1]);
        for(i; i<*nextAvailableP-(numberOfMails+2); i++){
            strcpy(contacts[i], contacts[i+numberOfMails+2]);
        }
        (*nextAvailableP)--;
        while(*nextAvailableP>=i){
            free(contacts[*nextAvailableP]);
            (*nextAvailableP)--;
        }
    }
    else{
        printf("Not on the list\n");
    }
}

//Gets the array, the number of mails to print and their first index and prints them numberd
void printXmailsNumberd(char* contacts[], int X, int beginIndex){
    int j;
    for(j=0; j<X; j++,beginIndex++){
        printf("%d. %s\n",j+1, contacts[beginIndex]);
    }
    printf("\n");
}
//Deletes a single mail address from a given user, if he exists, and rearrange the array afterwards
void deleteMailAddress(char* contacts[], int* nextAvailableP){
    int i = existReturnIndex(contacts, nextAvailableP);
    if(i != -1){
        i++;
        int numberOfMails = atoi(contacts[i]);
        int firstMailIndex = ++i;
        if(numberOfMails > 1){
            printXmailsNumberd(contacts, numberOfMails, i);
            printf("Choose the number of one of the above\n");
            int mailToDelete;
            scanf(" %d", &mailToDelete);
            for(i=mailToDelete+firstMailIndex-1; i<*nextAvailableP-1; i++){
                strcpy(contacts[i], contacts[i+1]);
            }
            itoa((numberOfMails-1),contacts[(firstMailIndex-1)], 10);
            (*nextAvailableP)--;
            free(contacts[*nextAvailableP]);
        }
        else{
            printf("Only one address available, you're not allowed to delete it\n");
        }
    }
    else{
        printf("Not on the list\n");
    }
}
//Checks if a name exits in the array and return his index
int existReturnIndex(char* contacts[], int* nextAvailableP){
    char *name = getString("Enter the contact name\n");
    if(name == NULL){
            printf("Malloc failed\n");
            return FALSE;
        }
    int i;
    for(i=0; i<*nextAvailableP; i++){
        if(strcmp(contacts[i],name) == 0){
           return i;
        }
    }
    free(name);
    return -1;
}
//prints the name of the person the user was looking for, and his mails, if he is on the list
void find(char *contacts[], int *nextAvailableP){
    int numberOfMails;
    int i = existReturnIndex(contacts, nextAvailableP);
    if(i != -1){
        i++;
        numberOfMails = atoi(contacts[i++]);
        printf("\n%s:\n", contacts[i-2]);
        printXmailsNumberd(contacts, numberOfMails, i);
    }
    else{
        printf("Not on the list\n");
    }
}

//Adds a contact to the list
int add(char *contacts[], int *nextAvailableP){
    if (*nextAvailableP > N-3){
        printf("Unable to add a contact, not enough memory\n");
        return FALSE;
    }

    int numberOfMails = 0;

    //receives the name, and saves it at the right cell
    contacts[*nextAvailableP] = getString("Enter the contact name\n");
    if(contacts[*nextAvailableP] == NULL){
        printf("Malloc failed\n");
        return FALSE;
    }
    (*nextAvailableP)++;

    //allocates the memory needed for the number of email addresses
    int numberOfMailsIndex = (*nextAvailableP);
    contacts[numberOfMailsIndex] = (char*)malloc(5);
    itoa(numberOfMails, contacts[numberOfMailsIndex], 10);
    (*nextAvailableP)++;

    //receives the name, and saves it at the right cell
    printf("Enter the mail/s, use enter to enter a mail and '-1' to signal you finished\n");
    while(1){
        if (*nextAvailableP > N-1){
            printf("Unable to add another mail, not enough memory\n");
            return FALSE;
        }
        char *tempMail = getString("Enter mail or '-1' to stop\n");
        if(tempMail == NULL){
            printf("Malloc failed\n");
            return FALSE;
        }
        if(strcmp(tempMail,"-1") == 0){
            break;
        }
        contacts[*nextAvailableP] = tempMail;
        printf("\nAdded!\n");
        (*nextAvailableP)++;
        numberOfMails++;
        itoa(numberOfMails, contacts[numberOfMailsIndex], 10);
    }
}

//Gets a string and prints it as a message.
//Then, read the text from stdin, saves it in a pointer and sends it back
char *getString(char *queryText){
char buffer[BUFFERSIZE];
char *temp;
printf("%s", queryText);

fgets(buffer, BUFFERSIZE, stdin);

//clears \n at the end of the line
if ((strlen(buffer) > 0) && (buffer[strlen(buffer) - 1] == '\n'))
    buffer[strlen(buffer) - 1] = '\0';

temp = (char*)malloc(strlen(buffer));
//if(temp == NULL)? , checked each time in the calling function
strcpy(temp, buffer);
return temp;}

Answer 1

Your code is not correct. It can crash.

First, look at the line

fgets(buffer, BUFFERSIZE, stdin);

If an error occurs while reading, the state of buffer is indeterminate. You should not continue processing it in that case. You need to detect this by the appropriate means and signal a failure.

Second, if the malloc() fails, what will be the effect of the next line which is

strcpy(temp, buffer);

hint: it's not good

To answer your specific questions:

1. Mostly
2. Yes
3. Your wording is imprecise. You're not changing the pointer, you're changing the memory pointed to by the pointer.
4. It is somewhat frowned upon to have a function allocate memory and then return a pointer to, because the caller may not know how the memory was allocated and how it needs to be deallocated (is it static? is it on the malloc heap? is it in a private buffer? is it in an OS-specific heap?). For a simple example what you have is probably okay but Best practice is for the caller to allocate it and then pass it in.