1

I am working on creating a system for entering student names and scores into arrays and printing the same information to the screen, unfortunately I keep getting the weird output.

I stepped through my program using a debugger and it showed that everything runs smooth until I get to the function that prints the student's information, there the double pointer char arrays have messed up values.

Here's an image of what I see when I run the program. (http://s28.postimg.org/nv29feawt/Error.png)

Note: Although I am aware that there are better and easier ways of doing this I am required to complete this assignment using dynamically allocated memory and arrays.

int main(void)
{
    char **firstNames;
    char **lastNames;
    float *scores;

    int recordsLength;
    printf("Please indicate the number of records you want to enter: ");
    scanf("%d", &recordsLength);
    printf("\n\n");

    firstNames = (char **)malloc(recordsLength * sizeof(char *));
    lastNames = (char **)malloc(recordsLength * sizeof(char *));
    scores = (float *)malloc(recordsLength * sizeof(float));

    int i = 0;
    while(i < recordsLength)
    {
        createNewEntry(i, firstNames, lastNames, scores);
        i++;
    }

    printEntry(0, firstNames, lastNames, scores);
    free(firstNames);
    free(lastNames);
    free(scores);
    return 0;
}

void clearScreen()
{
#ifdef _WIN32 
    system("cls");
#elif _unix_ 
    system("clear");
#endif
}

void printEntry(int entryID, char *firstNames[], char *lastNames[], float scores[])
{
    clearScreen();
    printf("|-------------------------------------------------------------------------|\n");
    printf("|                           Student Entry                                 |\n");
    printf("|-------------------------------------------------------------------------|\n|\n");
    printf("|   First Name: %s   Last Name: %s   Score: %.1f\n|\n|\n|\n", firstNames[entryID], lastNames[entryID], scores[entryID]);
    printf("|-------------------------------------------------------------------------|\n");
    printf("|                                                                         |\n");
    printf("|-------------------------------------------------------------------------|\n\n");
}

void createNewEntry(int index, char *firstNames[], char *lastNames[], float scores[])
{
    printf("Please input the records of the new student.\n\n\n");
    char first[20];
    char last[20];
    float score = 100.0f;

    printf("Please enter the student's first name: ");
    scanf("%s", &first);
    printf("\n\n");

    printf("Please enter the student's last name: ");
    scanf("%s", &last);
    printf("\n\n");

    printf("Please enter the student's score: ");
    scanf("%f", &score);
    printf("\n\n");

    firstNames[index] = (char *)malloc((strlen(first)) * sizeof(char));
    firstNames[index] = first;

    lastNames[index] = (char *)malloc((strlen(last)) * sizeof(char));
    lastNames[index] = last;
    printf("first name: %s", firstNames[index]);
    printf("last name: %s", lastNames[index]);
    scores[index] = score;
}
isbaran
  • 75
  • 1
  • 7
  • Never cast the returns from malloc realloc, calloc – Ivan Ivanovich Nov 14 '14 at 06:01
  • `firstNames[index] = first;` and `lastNames[index] = last;` look suspect. What you're doing is copying the pointer, not the array's contents. – bwinata Nov 14 '14 at 06:05
  • I hope you have include `stdio.h` in your original assignment. Moreover, it would be better if you cut down your code and post a shorter version of code (minimal code that demonstrates your issue. For example a version with only firstNames and no lastNames. – Mohit Jain Nov 14 '14 at 06:05
  • you need `strcpy` from char first[20]; to firstNames[index] and so on, because wheen your returned from `createNewEntry()` your `"firstNames[index]"` becomes invalid – Ivan Ivanovich Nov 14 '14 at 06:08
  • wheen your do this: `lastNames[index] = (char *)malloc((strlen(last)) * sizeof(char)); lastNames[index] = last;` you simply remove your pointer, that kept in the previous row – Ivan Ivanovich Nov 14 '14 at 06:11
  • Thank you, Ivan Ivanovich and Mohit Jain! It works like a charm now! Can I ask why I should never cast the returns from malloc realloc, and calloc? – isbaran Nov 14 '14 at 06:52
  • You can read more about this [here](http://stackoverflow.com/questions/953112/should-i-explicitly-cast-mallocs-return-value) – Mohit Jain Nov 14 '14 at 07:35

2 Answers2

1
firstNames[index] = (char *)malloc((strlen(first)) * sizeof(char));
firstNames[index] = first;  /* You are missing your allocated memory block and assigning local */

Above lines are incorrect. You can not assign c-strings with assignation = operator. You should use strcpy for that.

You are assigning local array to firstnames, which has no life after the function ends. This invokes an undefined behavior. (You are seeing the strange characters, but it can be even worse).

Should be Re-written as (Similar for last name also)

firstNames[index] = malloc((strlen(first) + 1) * sizeof(char)); /* +1 for \0 */
if(firstNames[index] == NULL) {
  /* Malloc failed, error handling */
  ...
}
/* else */
strcpy(firstNames[index], first);  /* Use strcpy to copy contents */

Live example here

Before freeing firstNames and lastNames, you should free all the member of firstNames and lastNames in a loop.

Community
  • 1
  • 1
Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
  • 1
    Or you can use `firstNames[index] = strdup(first);` — if necessary, writing `strdup()`. It is a POSIX function but not a Standard C function. Also, the code should check that `malloc()` succeeds before using the allocated memory. – Jonathan Leffler Mar 21 '15 at 06:13
0

I agree with the answer of Mohit Jain, adding to that you can even use sprintf.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Achyuta Aich
  • 569
  • 1
  • 5
  • 13
  • While you could use `sprintf()` — or even `snprintf()` — it is not clear why you would want to do so. You could use `memcpy()` or `memmove()` instead of `strcpy()` too, if you're careful to capture the length found by `strlen()` — that might be better. – Jonathan Leffler Mar 21 '15 at 06:15