Thread: Problems with Malloc

I'm trying to use malloc to increase the size of 3 dynamic arrays, but it keeps crashing as soon as it enters the first for loop. count is a pointer to an int holding the total nomber of records there should be, fname and lname are dynamic arrays holding names equal to count, and grades is another dynamic array holding the grades.

Code:

 void addRecs(int *count, char **fname, char **lname, float *grades)
{

  *count = *count + 1;

  //reallocate older stuff
  char **tempf = realloc(fname, *count * sizeof *tempf);
  for (int i = 0; i < *count - 1; i++) {
    tempf = realloc(fname, 20 * sizeof *tempf);
  }

  char **templ = realloc(lname, *count * sizeof *templ);
  for (int i = 0; i < *count - 1; i++) {
    templ = realloc(lname, 20 * sizeof *templ);
  }
  float *tempg = realloc(grades, *count * sizeof *tempg);

  //allocate space for newest
  templ[*count - 1] = (char *) malloc(20 * sizeof *lname[*count - 1]);
  tempf[*count - 1] = (char *) malloc(20 * sizeof *fname[*count - 1]);

  free(fname);
  free(lname);
  free(grades);

  fname = tempf;
  lname = templ;
  grades = tempg;

  printf("Add a record\n");
  scanf("%s %s %f", &fname[*count - 1][0], &lname[*count - 1][0],
        &grades[*count - 1]);
  int i;
  for (i = 0; i < *count; i++) {
    printf("First Name: %s, Last Name: %s, Score: %.2f \n", &fname[0],
           &lname[0], grades);
  }
}

F .

Originally Posted by jimblumberg

Have you considered using an array of a structure instead of what you're currently doing?

I wish I could but this is for an assignment so I'm required to use pointers. Here is the rest of the code:

Code:

#include<stdio.h>#include<string.h>
#include<stdlib.h>
#include<stdlib.h>

//prints all records in the same order they were entered
void printRecs(int *count, char **fname, char **lname, float *grades)
{
  printf("count: %d\n", *count);
  //for loop to print all the arrays
  int i;
  printf("count: %d\n", *count);

  for (i = 0; i < *count; i++) {
    printf("loop count: %d\n", i);
    printf("First Name: %s, Last Name: %s, Score: %.2f \n", &fname[0],
           &lname[0], grades);
  }
}

void addRecs(int *count, char **fname, char **lname, float *grades)
{

  *count = *count + 1;

  //reallocate older stuff
  char **tempf = realloc(fname, *count * sizeof *tempf);
  for (int i = 0; i < *count - 1; i++) {
    tempf = realloc(fname, 20 * sizeof *tempf);
  }

  char **templ = realloc(lname, *count * sizeof *templ);
  for (int i = 0; i < *count - 1; i++) {
    templ = realloc(lname, 20 * sizeof *templ);
  }
  float *tempg = realloc(grades, *count * sizeof *tempg);

  //allocate space for newest
  templ[*count - 1] = (char *) malloc(20 * sizeof *lname[*count - 1]);
  tempf[*count - 1] = (char *) malloc(20 * sizeof *fname[*count - 1]);

  free(fname);
  free(lname);
  free(grades);

  fname = tempf;
  lname = templ;
  grades = tempg;

  printf("Add a record\n");
  scanf("%s %s %f", &fname[*count - 1][0], &lname[*count - 1][0],
        &grades[*count - 1]);
  int i;
  for (i = 0; i < *count; i++) {
    printf("First Name: %s, Last Name: %s, Score: %.2f \n", &fname[0],
           &lname[0], grades);
  }
}

int main()
{
  int choice = 1;
  //ask user for total count of students, must be between 5 and 15
  int temp = 0;
  int *count = &temp;
  while ((*count > 15) || (*count < 5)) {
    printf
        ("Please indicate number of records you want to enter (min 5, max 15): ");
    scanf("%d", count);
  }

  //read in all the data for the students
  char **lname = (char **) malloc(*count * sizeof *lname);
  for (int i = 0; i < *count; i++) {
    lname = (char *) malloc(20 * sizeof *lname);
  }

  char **fname = (char **) malloc(*count * sizeof *fname);
  for (int i = 0; i < *count; i++) {
    fname = (char *) malloc(20 * sizeof *fname);
  }

  float *grades = (float *) malloc(*count * sizeof *grades);

  int i;
  printf
      ("Please input records of students (enter a new line after each record), with following format first name last name score\n");
  for (i = 0; i < *count; i++) {
    scanf("%s %s %f", &fname[0], &lname[0], &grades);
  }

  //display menu and switch inside a loop for selection
  //printf("Print records (press 1)\nSearch by first name (press 2)\nSearch by last name (press 3)\nSort by score (press 4)\nSort by last name (press 5)\nFind Max Score (press 6)\nFind Min Score (press 7)\nExit the program (press 0)\n");


  while (choice != 0) {
    printf("Enter choice: ");
    scanf("%d", &choice);
    switch (choice) {
    case 0:
      break;
    case 1:
      printRecs(count, fname, lname, grades);
      break;
    case 2:
      addRecs(count, fname, lname, grades);
      break;
    default:
      printf("Please only enter a number from 0 to 7\n");
      break;
    }

  }

}

Stop using such an awful font ffs.
Copy-as-text from your IDE, and/or paste-as-text in your browser.

Originally Posted by bandicoot

He specified the variables he wanted us to use and gave us these, and said we should not add any other arrays or structs.

I hope that this is merely a learning exercise in order to demonstrate as a negative example why this is a cumbersome way of doing things. If he does indeed write code like this for real himself, then he probably isn't a good programmer.

Now, onto to your code. What I suggest is this: start with a reduced version of the problem. For example, instead of a record consisting of first name, last name, and grade, pretend that each record consists of just a first name. Then write the code under this assumption. Compile and test. When you're pretty sure you have it working with just first names, include last name into the record. Modify the code under this assumption. Compile and test. When you're pretty sure you have it working with first names and last names, include grade into the record, etc.

This way, you'll be fixing just one problem at a time. If you've made a mistake with how you're handling first names, that's the mistake you have to fix. You don't have to replicate it for last names, then find out that your fix was wrong and you have to do over twice.

EDIT:
By the way, you don't have to do that "reallocate older stuff" thing. You do need to reallocate for the new record, but that's just one call of realloc for each item in the record (i.e., you reallocate the dynamic array for that item so that it can accommodate the new record), and then allocating space for the first and last names. You definitely do not need to call free when you add a record.

I don't know why you have an addRecs being called in your menu selection.

Aside from the initial allocation in main, and the subsequent input, there seems to be no reason to change the data.
So all of the malloc/realloc inside functions isn't necessary.

Besides, asking newbies to realloc a 2D array of pointers from within a function is a big ask.

Most of the rest was just fixing the array notation when trying to access your pointers.

Code:

#include<stdio.h>
#include<string.h>
#include<stdlib.h>

//prints all records in the same order they were entered
void printRecs(int *count, char **fname, float *grades)
{
  printf("count: %d\n", *count);
  //for loop to print all the arrays
  int i;
  for (i = 0; i < *count; i++) {
    printf("loop count: %d\n", i);
    printf("First Name: %s, Score: %.2f \n", fname[i], grades[i]);
  }
}

void addRecs(int *count, char **fname, float *grades)
{
  printf("Add a record\n");
  scanf("%s %f", fname[*count], &grades[*count]);
  *count = *count + 1;
  printRecs(count, fname, grades);
}

int main()
{
  int choice = 1;
  //ask user for total count of students, must be between 5 and 15
  int temp = 0;
  int *count = &temp;
  while ((*count > 15) || (*count < 5)) {
    printf
        ("Please indicate number of records you want to enter (min 5, max 15): ");
    scanf("%d", count);
  }

  //read in all the data for the students
  char **fname = malloc(*count * sizeof *fname);
  for (int i = 0; i < *count; i++) {
    fname[i] = malloc(20 * sizeof *fname[i]);
  }

  float *grades = malloc(*count * sizeof *grades);

  int i;
  printf("Please input records of students (enter a new line after each record), "
          "with following format first name last name score\n");
  for (i = 0; i < *count; i++) {
    scanf("%s %f", fname[i], &grades[i]);
  }

  //display menu and switch inside a loop for selection
  printf("Print records (press 1)\n"
         "Search by first name (press 2)\n"
         "Search by last name (press 3)\n"
         "Sort by score (press 4)\n"
         "Sort by last name (press 5)\n"
         "Find Max Score (press 6)\n"
         "Find Min Score (press 7)\n"
         "Exit the program (press 0)\n");

  while (choice != 0) {
    printf("Enter choice: ");
    scanf("%d", &choice);
    switch (choice) {
    case 0:
      break;
    case 1:
      printRecs(count, fname, grades);
      break;
    case 2:
      addRecs(count, fname, grades);
      break;
    default:
      printf("Please only enter a number from 0 to 7\n");
      break;
    }
  }
}

$ ./a.out 
Please indicate number of records you want to enter (min 5, max 15): 3
Please indicate number of records you want to enter (min 5, max 15): 5
Please input records of students (enter a new line after each record), with following format first name last name score
fred 1
barney 3
betty 4
wilma 10
bambam 0
Print records (press 1)
Search by first name (press 2)
Search by last name (press 3)
Sort by score (press 4)
Sort by last name (press 5)
Find Max Score (press 6)
Find Min Score (press 7)
Exit the program (press 0)
Enter choice: 1
count: 5
loop count: 0
First Name: fred, Score: 1.00 
loop count: 1
First Name: barney, Score: 3.00 
loop count: 2
First Name: betty, Score: 4.00 
loop count: 3
First Name: wilma, Score: 10.00 
loop count: 4
First Name: bambam, Score: 0.00 
Enter choice:

Oh, and if your compiler barfs on the uncast malloc with messages like "cant convert void* to...", then stop using C++ to compile your C program.

Thanks for looking over my code.

My current menu is wrong, I commented it out but forgot to remove it before posting my code here. I'm supposed to do a bunch of varying tasks, but this is the hardest one. I'm supposed to keep the variables I already have in there and then reallocate space so that the user can input another student after they input their original ones. I know its a bit of a dumb task, but its part of the assignment.

This is what the menu is supposed to have for context:

Print records (press 1)
Add a new record (press 2)
Delete record(s) (press 3)
Search by last name (press 4)
Sort by score (press 5)
Sort by last name (press 6)
Find median score (press 7)
Exit the program (press 0)

Are you expected to add more records than the initial 5 to 15?

Extending a 2D array using realloc is no small feat.
Because realloc can move the memory, you need to make sure you capture the possibly changed pointer.

If you're dealing with only one at once, you can use normal function pass by value and return.
But if you're trying to extend several parallel arrays all at once, it's messy!

Code:

#include <stdio.h>
#include <stdlib.h>


char **extender1( char **fname, int *count ) {
  char **tmp = realloc(fname, sizeof(*fname)*((*count)+1));
  if ( tmp ) {
    count += 1;
    return tmp;
  } else {
    return fname;
  }
}

// https://wiki.c2.com/?ThreeStarProgrammer
void extender2( char ***fname, int *count ) {
  char **tmp = realloc(*fname, sizeof(**fname)*((*count)+1));
  if ( tmp ) {
    count += 1;
    *fname = tmp;
  }

  // if you wanted to actually use the 2D array, it would be
  // (*fname)[i]
}

int main()
{
  int count = 10;

  //read in all the data for the students
  char **fname = malloc(count * sizeof *fname);
  for (int i = 0; i < count; i++) {
    fname[i] = malloc(20 * sizeof *fname[i]);
  }

  // extend inline
  char **tmp = realloc(fname, sizeof(*fname)*(count+1));
  if ( tmp ) {
    fname = tmp;
    count += 1;
  }

  // extend via a function, one way
  fname = extender1( fname, &count );

  // extend via a function, another way
  extender2( &fname, &count );
}