-5

My code should print to top score entered for each student, but instead for each student it reprints the top score of all the students.

Can anyone explain why? What's the error...even working it out on paper I don't see it.

//Take integer marks (0-100) of 3 tests of 4 students and print the highest score of respective student

#include<stdio.h>

int main (void)
{
    int i=0, j=0, a=0, b=0;
    int
        arr[a][b],
        max[i];




    printf("Enter the number of students, and number of tests score per student: \n\n");
    scanf("%d", &a);
    scanf("%d", &b);
    //int arr[][] = arr[a][b];
    for(i=0; i<a; i++)
        {
        printf("Enter the scores for student %d \n", (i+1));
            for(j=0; j<b; j++)
            {
            scanf("%d", &arr[i][j]);
            }
        }

    for(i=0; i<a; i++)
        {
        max [i] = 0;
        for(j=0; j<b; j++)
            {
            if(arr[i][j]>max[i])
                {
                max[i] = arr[i][j];
                }
            }
        }

     for(i=0; i<a; i++)
     {
         printf("The highest score for student %d was:\t %d\n\n", (i+1), max[i]);
     }

}
Srini
  • 1,619
  • 1
  • 19
  • 34
Noel Nosse
  • 31
  • 5
  • 5
    The dimensions of your arrays are all 0 at the time they are declared. – Scott Hunter Apr 12 '18 at 17:28
  • 1
    Try changing your array declarations to something like this: `int arr[MAX_STUDENTS][MAX_SCORES], max[MAX_STUDENTS];` Where you previously defined those constants to some suitable numbers, e.g. `#define MAX_STUDENTS 100`, and `#define MAX_SCORES 50`. – bruceg Apr 12 '18 at 17:30
  • or move the declaration `int arr[a][b];` after your `scanf` statements – Tormund Giantsbane Apr 12 '18 at 17:35

1 Answers1

2

Compiling your code produces no warnings, good work, but running it with -fsanitize=address immediately reveals a problem.

$ make
cc -fsanitize=address -Wall -Wshadow -Wwrite-strings -Wextra -Wconversion -std=c99 -pedantic -g `pkg-config --cflags glib-2.0`   -c -o test.o test.c
cc `pkg-config --libs glib-2.0` -lssl -lcrypto -fsanitize=address  test.o   -o test

$ ./test
Enter the number of students, and number of tests score per student: 

3
3
Enter the scores for student 1 
5
=================================================================
==80655==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffeed7491e0 at pc 0x0001028420c3 bp 0x7ffeed749060 sp 0x7ffeed748820
WRITE of size 4 at 0x7ffeed7491e0 thread T0
10    #0 0x1028420c2 in scanf_common(void*, int, bool, char const*, __va_list_tag*) (libclang_rt.asan_osx_dynamic.dylib+0x1a0c2)
    #1 0x10284235e in wrap_scanf (libclang_rt.asan_osx_dynamic.dylib+0x1a35e)
    #2 0x1024b76e0 in main (/Users/schwern/tmp/./test+0x1000016e0)
    #3 0x7fff5d504014 in start (/usr/lib/system/libdyld.dylib+0x1014)

The problem is here.

int i=0, j=0, a=0, b=0;
int arr[a][b], max[i];

arr is initialized as arr[0][0] and never extended. When you try to write to arr[i][j] with scanf("%d", &arr[i][j]) the memory will not be allocated. Instead the program will overwrite some other memory and strange things will happen. Instead, arr will have to be initialized after we get the sizes. Similar issue with max.

This class of errors is often a culprit of doing all your variable declaration and initialization at the start of the function. This is a style you'll see a lot, but it's no longer necessary unless you're using a very ancient compiler.

Instead, declare your variables in context. Then it's obvious what they're for. And only initialize them when necessary, then you can get "uninitialized value" warnings from the compiler.


To fix this we could allocate memory for arr and max, but allocating a 2D array like arr is not simple.

Instead we can observe that all your outer loops iterate over the same thing: the number of tests. There's no need to store all student's test scores and then calculate the max. We can do each student in one loop. No arrays needed to store data between loops.

#include <stdio.h>

int main (void) {
    printf("Enter the number of students, and number of tests score per student: \n\n");

    // Variables are declared as needed with names describing
    // what they are to make the code self-documenting.
    // They are not unnecessarily initialized allowing warnings to help out.
    int num_students;
    int tests_per_student;
    scanf("%d", &num_students);
    scanf("%d", &tests_per_student);

    // Each student is fully processed in a single loop. No arrays needed.
    // Note the other variables declared in place in as narrow a scope as possible.
    for(int i=0; i < num_students; i++) {
        // By declaring `max` inside the loop it is guaranteed to be reset each
        // loop iteration. Code outside the loop cannot affect it. It cannot
        // be confused with another `max`.
        int max = 0;

        printf("Enter the scores for student %d \n", (i+1));
        for(int j=0; j < tests_per_student; j++) {
            // Similarly for score. We only need it inside this one loop.
            int score;
            scanf("%d", &score);

            if(score > max) {
                max = score;
            }
        }

        // With all the work done, print the max for this student and move on
        // to the next one.
        printf("The highest score for student %d was:\t %d\n\n", (i+1), max);
    }
}
Schwern
  • 153,029
  • 25
  • 195
  • 336