Strange output while using dynamic memory allocation i C

Question

I'm working on this code, and it's not finished yet. But while testing the add new contacts function and printing function, I get this strange output. Everythin looks ok as long as I'm not adding more than one contact. What am I doing wrong here?

//Libraries:
#include <stdio.h>
#include <stdlib.h>

//Function prototypes
int showMenu(); //Shows user options

//Main function
int main() {

    //Variable declerations
    int i = 0;
    int iOption = 0;
    int iContacts = 0;
    char *cPhoneBook_name;
    int *iPhoneBook_nr;

    cPhoneBook_name = (char *) calloc(1, 80 * sizeof(char));
    iPhoneBook_nr = (int *) calloc(1, sizeof(int));

    do {

        //Memory check
        if(cPhoneBook_name == NULL || iPhoneBook_nr == NULL){
            printf("\nOut of Memory!\n");
            return;
        }

        //Show menu
        iOption = showMenu();

        switch (iOption) {
            case 1: //Add contact
                printf("\nEnter name:\n");
                scanf("%s", &cPhoneBook_name[iContacts]);
                printf("\nEnter number:\n");
                scanf("%d", &iPhoneBook_nr[iContacts]);
                cPhoneBook_name = realloc(cPhoneBook_name, 80 * sizeof(char));
                iPhoneBook_nr = realloc(iPhoneBook_nr, sizeof(int));
                iContacts += 1;
                break;
            case 2: //Modify contact
                //Code here
                break;
            case 3: //Show contacts
                for(i = 0 ; i < iContacts ; i++) {
                    printf("\n%s %d\n", &cPhoneBook_name[i], iPhoneBook_nr[i]);
                }
                break;
            case 4: //Free memory (delete contacts)
                free(cPhoneBook_name);
                free(iPhoneBook_nr);
                break;
        }
    }while(iOption != 5);//Exit if iOption is 5;

    system("clear");
    return 0;
}


//Function definition - showMenu()
int showMenu() {

    int iOption = 0;

    //system("clear");

    printf("\n   Phone book   \n");
    printf("------------------------\n");
    printf("1. Add new contact.\n");
    printf("2. Edit existing contact\n");
    printf("3. Show contact(s)\n");
    printf("4. Clear phone book\n");
    printf("5. Exit\n");
    printf("------------------------\n");
    printf("Option --> ");
    scanf("%d", &iOption);

    return iOption;
}
//---------------------------------------------------------

Show us an example of this "strange output", otherwise we're just guessing. — John Bode, Jun 03 '15 at 15:23
Why are you allocating memory and not putting anything into it? — Lee Daniel Crocker, Jun 03 '15 at 15:28
Your main problem is that your scanf's are going into unallocated memory. — Lee Daniel Crocker, Jun 03 '15 at 15:30
Take a look here: [dynamic memory for 2D char array](http://stackoverflow.com/questions/2614249/dynamic-memory-for-2d-char-array). — raymelfrancisco, Jun 03 '15 at 15:37

David van rijn · Accepted Answer · 2015-06-03T15:36:58.220

2

The problem lies within this:

printf("\n%s %d\n", &cPhoneBook_name[i], iPhoneBook_nr[i]);

cPhoneBook_name if of type char[]. So for example the expression char[1] gives you the second charachter in the list. While a name in your code is 80 chars wide. So try:

printf("\n%s %d\n", &cPhoneBook_name[i*80], iPhoneBook_nr[i]);

Edit:

I also found that this:

cPhoneBook_name = realloc(cPhoneBook_name, 80 * sizeof(char));

Has no effect, because you're resizing cPhoneBook_name from 80 to 80. try:

    cPhoneBook_name = realloc(cPhoneBook_name, (iContacts+1) 80 * sizeof(char));

and also: scanf("%s", &cPhoneBook_name[80*iContacts]);

edited Jun 03 '15 at 15:36

answered Jun 03 '15 at 15:28

David van rijn

2,108
9
21

This line is also having no effect. `iPhoneBook_nr = realloc(iPhoneBook_nr, sizeof(int));` and failing to extend the array of numbers. – Persixty Jun 03 '15 at 15:41
Good find :). So in general just remember `pointer = realloc(pointer*newItemCount*sizeof(type))`. – David van rijn Jun 03 '15 at 15:44
2

Ahem, `pointer = realloc(pointer , newItemCount*sizeof(type))`. – Persixty Jun 03 '15 at 15:54
1

Haha, oops, i'll go make coffee ;) – David van rijn Jun 03 '15 at 15:56
Thanks @Davidvanrijn ! It helped a lot! I finished the code with your help. :-) – user2722928 Jun 03 '15 at 17:20

score 2 · Answer 2 · answered Jun 03 '15 at 16:08

This is most likely the root of your "strange ouput" when you enter more than one contact:

cPhoneBook_name = realloc(cPhoneBook_name, 80 * sizeof(char));

You're not actually extending the size of the cPhoneBook_name array; in a realloc call, the second parameter is the new size of the array, not the amount to add to the existing size.

Having said that, this whole approach is wrong; you're not creating an array of strings, you're (unsuccessfully) creating a single string that gets extended every time you add a contact, and your iContacts index will point to the i'th character of the string, not the i'th string in the list of strings.

Here's an approach that should work:

#define ROWS 5 // start with 5 rows in your contact list

// Create a type to store a single phone book entry
struct PhoneBookEntry {
  char name[80];  // fixed size to keep things simple
  int number;
};

// Create an array that can initially store ROWS
// phone book entries
struct PhoneBookEntry *phoneBook = malloc ( sizeof *phoneBook * ROWS );
size_t phoneBookRows = ROWS;
...
case 1: // Add contact
  // Before adding a contact, see if we need to extend the phone book
  // array
  if ( iContacts == phoneBookRows )
  {
    // double the size of the phone book array
    struct PhoneBookEntry *tmp = realloc( phoneBook, 2 * sizeof *phoneBook * phoneBookRows );
    if ( tmp )
    {
      phoneBook = tmp;
      phoneBookRows *= 2; 
    }
    else
    {
      fprintf( stderr, "Could not extend phone book!\n" );
      break;
    }
  }
  ...
  scanf( "%s", phoneBook[iContacts].name ); // note no & operator; arrays are special
  ...
  scanf( "%d", &phoneBook[iContacts].number );

Some things to note:

First, DO NOT cast the result of malloc, calloc, or realloc in C code; it's unnecessary, and under C89 compilers can mask a bug. You do need to cast the results of those functions in C++ code, but if you're writing C++ you shouldn't be using malloc, calloc, or realloc anyway.

Second, avoid using type names as sizeof arguments where you can help it. Use something like the following instead:

T *p = malloc( N * sizeof *p );

The type of the expression *p is T, so sizeof *p is equivalent to sizeof (T). This way, malloc will always allocate the correct amount of memory regardless of T, and if you ever decide to change T, you only have to make that change in one place.

Third, realloc will return NULL if it can't satisfy the request; if you assign that NULL to your phoneBook variable, you will lose your reference to the memory you've already allocated. It's safer to assign the result of realloc to a temporary variable, test for NULL, and then assign it to your phoneBook variable.

Fourth, realloc calls can be expensive, so you want to minimize them when it makse sense to do so. Doubling the size of the array is a common technique.

Strange output while using dynamic memory allocation i C

2 Answers2

Edit: