0

New picture from an external compiler.. the exit code is ok?

enter image description here

This is the full code. I'm having a trouble program blows away after printing the wanted output to the screen. I guess it's a problem with the way I allocated memory for the array of structs, and the .name field of each struct in a for loop.

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

#define MAX_NAME_LEN 50

typedef struct stud
{
char *name;
int marks[4];
float avg;
}student;


student* Create_Class(int);
void Avg_Mark(student*);
void Print_One(student*);
void printExcellent(student*);

void main()
{
int size, i;
student *arr, *newArr;
printf("\nEnter the number of students: ");
scanf_s("%d", &size);
newArr = Create_Class(&size);
for (i = 0; i < size; i++)
{
    printExcellent(newArr+i);
}
for (i=0;i<size;i++) free(newArr[i].name);
free(newArr);
_getch();
}

student* Create_Class(int size)
{
student *p;
char str[MAX_NAME_LEN];
int i, j;
p = (student*)calloc(size , sizeof(student));
if (!p)
{
    printf("Memory allocation failure.");
    exit(1);
}

for (i = 0; i < size; i++)
{
    printf("Enter your name: ");
    rewind(stdin);
    gets(str);
    p[i].name = (char*)calloc(strlen(str)+1,sizeof(char));
    if (!(p[i].name))
    {
        printf("Memory allocation error!");
        exit(1);
    }
    strcpy_s(p[i].name,50,str);
    printf("Enter your marks: ");
    for (j = 0; j < 4; j++)
    {
        scanf_s("%d", &p[i].marks[j]);
    }
    Avg_Mark(p + i);
}
return p;
}


void Avg_Mark(student* s)
{
int i, sum=0;
for (i = 0; i < 4; i++)
    sum += s->marks[i];
s->avg = (float)sum / 4;
}


void Print_One(student* s)
{
printf("The average of %s is %.1f\n", s->name, s->avg);
}

void printExcellent(student* s)
{
if ((s->avg) > 85)
    Print_One(s);
}
halfer
  • 19,824
  • 17
  • 99
  • 186
  • 3
    You missed to post the function code. – Ctx Apr 02 '19 at 00:22
  • Post your code so we can see what's going on. It sounds like it's a memory leak, but hard to tell cause there's no code to look at. – alex067 Apr 02 '19 at 00:23
  • ya just done so, sry – Meni Shitrit Apr 02 '19 at 00:23
  • 2
    Please post your full code, probably the insufficient allocation of `str` is the culprit. `gets()` is kinda evil anyway, and rewind(stdin) is a no-op _in the best case_ – Ctx Apr 02 '19 at 00:25
  • just done so, took some time but there is the full one – Meni Shitrit Apr 02 '19 at 00:31
  • 1
    `student* Create_Class(int);` and `newArr = Create_Class(&size);` ??? Is the argument you pass really correct? – Some programmer dude Apr 02 '19 at 00:39
  • also, why are you rewinding stdin? – bruceg Apr 02 '19 at 00:43
  • The code posted in the question isn't the same code shown in the screen shot. There, you use `gets_s()` but here you use `gets()` which strangely is right before where the exception error occurred. Which was it? Please read [Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used) – Weather Vane Apr 02 '19 at 00:44
  • 5
    Rule of thumb is: first eliminate _all_ warnings, then ask for help. – Ctx Apr 02 '19 at 00:44
  • hi, I tried some new things in order to maybe make it work. I fixed the paramter passed to the func Create_Class, now its just size without &. I have 0 warnings on this program. I wouldnt be asking for help if I knew another way to get a string. scanf %s doesnt work either. – Meni Shitrit Apr 02 '19 at 00:45
  • You aren't using `scanf` with `%s` in the code posted. If you are using `scanf_s` with `%s` in the different code, please be aware that, unlike `scanf`, it also needs a **size** paramater passed. – Weather Vane Apr 02 '19 at 00:51
  • The proactive approach to resolve this would be to eliminate all the warnings first, then start stripping out parts of the program until you get it to work and then add parts back in getting each part working before adding back more. – Paul Rooney Apr 02 '19 at 00:57
  • it is clear in the picture that there are 0 warnings. or the settings are too easy with me? – Meni Shitrit Apr 02 '19 at 00:58
  • 1
    As I wrote: it's not the same code as you posted. You should be able to adjust the settings level from the menus somewhere. – Weather Vane Apr 02 '19 at 00:59
  • yes. You have your settings too lenient. If there are no warnings for this code, you should worry. `newArr = Create_Class(&size);` alone is very wrong, its passing a pointer where you should pass an integer. – Paul Rooney Apr 02 '19 at 00:59
  • This one's fixed. what more? – Meni Shitrit Apr 02 '19 at 01:01
  • With that change is there still an issue? Are any of the strings you enter too large for the buffers you use? Try using a debugger. – Paul Rooney Apr 02 '19 at 01:08
  • ya still the same issue. no idea about the last two things you wrote. – Meni Shitrit Apr 02 '19 at 01:17

1 Answers1

0

Gonna point out everything fishy I see for you:

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

#define MAX_NAME_LEN 50

typedef struct stud
{
    char *name;
    int marks[4];
    float avg;
}student;


student* Create_Class(int);
void Avg_Mark(student*);
void Print_One(student*);
void printExcellent(student*);

void main()
{
    int size, i;
    student *arr, *newArr;
    printf("\nEnter the number of students: ");
    scanf_s("%d", &size);

    // This is wrong. Remove the &...
    newArr = Create_Class(&size);

    for (i = 0; i < size; i++)
    {
        printExcellent(newArr+i);
    }
    for (i=0;i<size;i++) free(newArr[i].name);
    free(newArr);
    _getch();
}

student* Create_Class(int size)
{
    student *p;
    char str[MAX_NAME_LEN];
    int i, j;

    // Consider checking size for a sane value.

    // Ok, allocate an array of students.
    p = (student*)calloc(size , sizeof(student));
    if (!p)
    {
        printf("Memory allocation failure.");
        exit(1);
    }

    for (i = 0; i < size; i++)
    {
        printf("Enter your name: ");

        // These 2 lines scare the heck out of me.  I'd really do this differently.
        // gets is the devil and the see:
        // https://stackoverflow.com/questions/20052657/reversing-stdin-in-c
        // for why this may not work well.

        rewind(stdin);
        gets(str);

        // What if str is not a terminated string?  Then 1 char of 0?  Guess this is ok.  Hope it doesn't overflow on the copy below though (consider fixed max size and not using a temporary)
        p[i].name = (char*)calloc(strlen(str)+1,sizeof(char));
        if (!(p[i].name))
        {
            printf("Memory allocation error!");
            exit(1);
        }
        // Do a fast copy of up to 50 chars. I'd really want to verify this output to be sure it works. 
        strcpy_s(p[i].name,50,str);
        printf("Enter your marks: ");
        for (j = 0; j < 4; j++)
        {
            // Hope this inputs the way you want.
            scanf_s("%d", &p[i].marks[j]);
        }

        // This should work, but I prefer more explicit pointers.
        Avg_Mark(p + i);
    }

    return p;
}


void Avg_Mark(student* s)
{
    // What if s is Null?

    int i, sum=0;

    // 4 is a magic number.  Make this a constant.
    for (i = 0; i < 4; i++)
        sum += s->marks[i];

    // This won't be as accurate as you want.  Consider an integer solution.
    s->avg = (float)sum / 4;
}


void Print_One(student* s)
{
    // What if s is Null?  What about s->name?
    printf("The average of %s is %.1f\n", s->name, s->avg);
}

void printExcellent(student* s)
{
    // What if s is Null?
    if ((s->avg) > 85)
        Print_One(s);
}

Note: While going through this code, I did not see any "red flags" except for the & on the size and perhaps the gets/rewind calls. I'd still add null asserts to your functions and also walk through it with a debugger to be sure that everything is as you expect. Honestly, there is enough going on here that I'd prefer the debugger help to my quick trace of the code while I was writing comments.


Update

If I change all your scanf_s to scanf() calls, replace your gets() / rewind() calls to a simple scanf("%s", str) call, and change your funky strcpy_s() function to a simpler strcpy() or strncpy() call, your program does not seem to crash for me. My money is that the strcpy_s() call is corrupting RAM while doing its "fast" copy.

Michael Dorgan
  • 12,453
  • 3
  • 31
  • 61
  • thanks so much for actually analyzing the code for me. a few questions if you will : 1.why the average won't be accurate? 2.what do you mean by more "explicit" pointers? instead of sending p+i sending &p[i]?? and last question, 3. without the rewind(stdin) the program prints enter name : and marks with no chance for the user to enter his name to the buffer. thanks for all the help ! – Meni Shitrit Apr 02 '19 at 02:28
  • 1. A floating point value here may display 1.2499994787 instead of 1.25 for an average. Something for later though. 2. Yes, something like that. I doubt this is where the problem is, but I personally am able to read `&newArr[i]` easier than the add - this is a preference though. 3. This sounds like your previous scanf didn't get all the input from stdin last time? I avoid scanf like the plague and would ask someone else here how fix this. – Michael Dorgan Apr 02 '19 at 02:41
  • And for gets() - https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used – Michael Dorgan Apr 02 '19 at 02:42
  • Finally, compile this with -Wall and fix any remaining warnings to be sure I didn't miss anything else "easy" – Michael Dorgan Apr 02 '19 at 02:44
  • 1
    Compile with `-Wall -Wextra -pedantic` (for gcc/clang) or `/W3` (for VS/`cl.exe`) and fix ALL warnings. Do not accept code until it compiles without a single warning. – David C. Rankin Apr 02 '19 at 03:19
  • SOLVED. The problem was with my understanding of the way function "strcpy_s" works. I entered the destination, source, and another paramater which I thought represent the max chars to copy. so I entered 50. There IS NO WAY to write 50 chars to destination which his size is strlen(str). therefore, instead of 50, i passed to strcpy_s, alongside the dest and source, the size strlen(str)+1 which is the right amount of chars to copy. now it's working. thanks everyone for the assistance. – Meni Shitrit Apr 02 '19 at 07:46