-1

My program is to allow user to input no. of students and their preferred sessions, then sort all of them by the session with most students.

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

struct student {  //*** PART 2 STRUCTURE DEFINITION **********
    char studName[50];  // Name of student
    long long int studID;        // Student ID of student
    int session;      // Prefered session of each student
};

struct session {
    int sessionID;
    int NoofStudinSession;
};

FILE * fPointer;


void main(void)
{
    //  Definitions
    struct student *myStudents;
    struct session *sessionsPtr;
    int Noofsessions = 0;
    int NoofStud = 0;
    int i;

    //  User input for number of students and consultation sessions
    printf("Enter Number of Students:\n");
    scanf("%d", &NoofStud);
    printf("Enter number of consultation session:\n");
    scanf("%d", &Noofsessions);

    //  Dynamic allocation for struct
    myStudents = (struct student *) malloc(sizeof(struct student) * NoofStud);

    if (myStudents == NULL) {
        printf("Error, out of memory.\n");
        return;
    }

    sessionsPtr = (int *)malloc(sizeof(struct session) * Noofsessions);

    if (sessionsPtr == NULL) {
        printf("Error, out of memory.\n");
        return;
    }

    // Initializing struct session before using
    for (i = 0; i < Noofsessions; i++) {
        (sessionsPtr + i)->sessionID = i + 1;
        (sessionsPtr + i)->NoofStudinSession = 0;

    }

    //  User input display
    for (i = 0; i < NoofStud; i++) {
        printf("Enter Student %d ID: \n", i + 1); // Prompt user to enter the ID of students
        scanf("%d", &(myStudents + i)->studID); //**


        printf("Enter Student %d Name: \n", i + 1); // Prompt user to enter the Student name
        scanf(" %[^\t\n]s", &(myStudents + i)->studName); //**

        printf("Enter Student %d's prefered Session(Number of session(s) is %d):\n", i + 1, Noofsessions); // Prefered Session
        scanf("%d", &(myStudents + i)->session); //**

        (sessionsPtr + ((myStudents + i)->session - 1))->NoofStudinSession++;
    }


    //  Display summary of Part2 on screen
    printf("Student ID\t Student Name\t\t Chose Which Session\n");
    fprintf(fPointer, "Student ID\t Student Name\t\t Chose Which Session\n"); // Write into Text file
    for (i = 0; i < NoofStud; ++i) {

        printf("%d\t  %15s\t\t\t %d\n", (myStudents + i)->studID, (myStudents + i)->studName, (myStudents + i)->session);
        fprintf(fPointer, "%d\t  %15s\t\t\t %d\n", (myStudents + i)->studID, (myStudents + i)->studName, (myStudents + i)->session);
    }

    // Sorting of sessions from MOST students to LEAST students
    int total = 0, swapped, pos;

    //comparing and sort values
    while (1) {
        pos = 0;

        for (i = 0; i < Noofsessions - 1; ++i) {

            if ((sessionsPtr + i)->NoofStudinSession < (sessionsPtr + i + 1)->NoofStudinSession) {

                swapped = sessionsPtr;
                sessionsPtr = sessionsPtr + 1;
                swapped = sessionsPtr + 1;
                pos = 1;

            }
        }
        if (pos == 0) {
            break;
        }
    }

    //  Display the Sorted Table
    printf("\nSession\t\t\t No. of Students\n");
    fprintf(fPointer, "\nSession\t\t\t No. of Students\n"); // Write into Text file

    for (i = 0; i < Noofsessions; i++) {

        printf("%d\t\t\t    %d\n", (sessionsPtr + i)->sessionID, (sessionsPtr + i)->NoofStudinSession);
        fprintf(fPointer, "%d\t\t\t    %d\n", (sessionsPtr + i)->sessionID, (sessionsPtr + i)->NoofStudinSession); // Write into Text file
    }

    free(myStudents);
    free(sessionsPtr);

    system("pause");
    return 0;

}

When the Noofsession is to be smaller than 3, it runs smoothly. However when it goes higher, this error occurs (HEAP error Invalid address specified to RtlValidateHeap).

EDIT: to better explain the problem, here is the display right after crashing. Output Correct me if I'm wrong, so the output at the sorted tables are actually the address? instead of the sorted values.

  • 1
    `sessionsPtr = (int *)malloc(sizeof(struct session) * Noofsessions);` - what is that supposed to mean? Why are you casting the result of `malloc`? And why are you casting it to `int *` specifically? What does `int *` have to do with anything here? – AnT stands with Russia Nov 28 '17 at 16:59
  • 1
    Please enable all warnings and treat them as errors – Jabberwocky Nov 28 '17 at 17:00
  • 1
    What is `scanf(" %[^\t\n]s"...`? What did you try to achieve when you put that `s` after the `]` there? – AnT stands with Russia Nov 28 '17 at 17:01
  • When do you get the HEAP error? Right at the beginning? When you start entering certain data? When? And what data did you input? What's the purpose of `swapped`? Did you try using a debugger to step through? – lurker Nov 28 '17 at 17:11
  • The error occurs when the program starts to sort the data, everything goes smoothly till the sorting, they even display the first output where it prints all the inputs by the admin before sorting – Jaden Chong Nov 28 '17 at 17:13
  • Sorry, but i am still pretty new to this. From what i understand the (int *) is a way to define the data that will be entered??. gosh i dont even know how to make the words i type into codes like you guys – Jaden Chong Nov 28 '17 at 17:15
  • 1
    `int *` means pointer to `int`. If you aren't entering an `int` then it's the wrong pointer to use. And most certainly you wouldn't cast `malloc` and definitely not as `int *` when assigning it to a `struct session *` pointer. As was mentioned, turn on warnings and work through resolving them. A warning is the compiler saying, "Are you sure you know what you're doing here?". – lurker Nov 28 '17 at 17:17
  • 1
    `sessionPtr = sessionPtr + 1` keeps increasing `sessionPtr`. Then later in your code, you are accessing, for example, `(sessionsPtr + i)->sessionID` as if `sessionPtr` was what you originally allocated. That's going to go out of range in memory access. Also, you ultimately do `free(sessionsPtr);` after changing `sessionPtr`. You *must* free the original pointer. You can't change it then expect `free` to make sense out of it. That's probably where your HEAP error came from. – lurker Nov 28 '17 at 17:28
  • for the `sessionPtr = sessionPtr + 1` its used for bubble sorting, so im actually wanting the `sessionPtr` at position _i_ to swap position with `sessionPtr` at position _i+1_. So i shouldnt be using `sessionPtr = sessionPtr + 1`? – Jaden Chong Nov 28 '17 at 17:33
  • `sessionsPtr + i` is much more readable if you write it as `sessionsPtr[i]` – Chris Turner Nov 28 '17 at 17:34
  • I understand you're trying to do a bubble sort swap, but it's totally discombobulated. Examine your swap logic carefully. You aren't using your `swapped` variable at all and it's the wrong type for how you're trying to use it. And you *must not* change the value of `sessionPtr`. – lurker Nov 28 '17 at 17:44
  • `(sessionsPtr + ((myStudents + i)->session - 1))->NoofStudinSession++;` Hmm.... you have no control where `(sessionsPtr + ((myStudents + i)->session - 1))` points... the user can give you any kind of input an crash your program – Support Ukraine Nov 28 '17 at 17:53
  • Somehow I have solved the problem and thank you so much. To further clarify, is it right for me to say that I've only been swapping the `sessionsPtr` instead of the variable its pointing to `NoofStudinSession`? I've only changed the bubble sorting `swapped` variables to `(sessionsPtr + i)->NoofStudinSession` and it worked! – Jaden Chong Nov 28 '17 at 17:56
  • @4386427, Yeahh I know of this problem but currently i have not learnt to that extent of coding yet. Wouldnt mind you giving me afew tips on how i can control the input by the user to further improve on this program. Thanks in advance! – Jaden Chong Nov 28 '17 at 17:58
  • Beware. Your program might run and appear to provide correct results but could still be quite faulty. For example, even if you access memory incorrectly with a pointer, the system might let you do it as it may still be within your program's entire allocated space, but just not the right space. That could lead to a failure sometimes under some conditions. Examine your memory allocations, usage, and de-allocations carefully. Find an experienced programmer in your class or an instructor to review your code with you. Or try https://codereview.stackexchange.com/. – lurker Nov 28 '17 at 18:07
  • Yup, I just realised that the program didnt move the `sessionID` together with `NoofStudinSession` when sorting... I guess i'll leave it for tomorrow. Still thanks for the advice! – Jaden Chong Nov 28 '17 at 18:14

1 Answers1

-1

Try to avoid multiplying inside calls to malloc. When allocating arrays of structures, use calloc.

That said, heed Michael Walz's comment. Get rid of all your warnings.

nmichaels
  • 49,466
  • 12
  • 107
  • 135
  • 2
    This I hear for the first time: avoid multiplying inside calls to malloc? Why? – Ctx Nov 28 '17 at 18:10
  • 1
    @Ctx: The point is valid (https://stackoverflow.com/a/1586016/187690), but to claim that this is a sufficient reason to "avoid multiplying inside calls to `malloc`" is preposterous. It is just something to keep in mind when multiplying (inside `malloc` or anywhere). – AnT stands with Russia Nov 28 '17 at 19:07
  • `calloc` can catch multiplication overflows and return an error. `malloc` can't. Particularly when you're allocating runtime-defined (or user-defined, which should probably be checked) numbers of elements, it's safer to have calloc check the multiplication and fail if it overflows. The standard library implementers are more careful than you are. As to the idea that you should always keep multiplication overflows in mind, the number of pitfalls in C is such that there are too many things to always keep in mind. – nmichaels Nov 28 '17 at 19:43