Alternative grading scheme

grading_scheme.md

Grading scheme proposal

Idea

The TA writes down everything (s)he sees no matter how important it is and doesn't give any "point". The actual grade is computed from the list of issues collected from the comments.

The issues are not specific to the subject rather they are generic comments that can be made about any piece of C code. The issues are codified using a universal issue list. There is little to no room left for interpretation. A direct consequence is that the grading must be as fair* as possible.

* Fairness here means that the handouts should end up sorted by subjective quality based. There is still a subjectiveness bias but it applies to all handouts equally and is not dependent on which TA graded it.

In practice

For each issue, the TA must add a tag in comment which contains the error code and the context. Two reference grid give the codes for the error types and for the contexts.

See the examples section for more info.

Issues list

This is the universal, exhaustive list of all possible mistakes that can be made in a C project. If you can't find what you are looking for in there, suggest an addition.

Style

• [E:NOCOMMENT] missing comment: A piece of code is hard to understand and no comment is there to help.
• Doxygen comment bloc
  • [E:DOX_NOCOMMENT] missing
  • [E:DOX_WRONG] inaccurate: The comment doesn't accurately describe the prototype (eg. bad argument name)
• [E:INDENT] bad indentation: (Once per chunk of instructions that belong together, like a function or part of a function left to the TA's appreciation)

Algorithms

• overkill algorithm: too many edge cases, bad logic or bad approach of the problem
  • [E:OVKL_C] correct: The algorithm is correct despite being overkill
  • [E:OVKL_W] wrong: The algorithm gives bad results
• [E:EDGE] missing edge case: When the algorithm works except for edge cases (n = 0, n = max, ...)
• [E:NONDET_JUMP] nondeterministic jump: A jump depends on an unitialized/undetermined value
• [E:INIT] passing uninitialized value: Similar to nondeterministic jump but for when the value is passed to an API outside our control
• [E:ULESS_JUMP] useless jump: An if/while or another jump control instruction that does not contribute to solving the problem
• [E:CONST_JUMP] constant conditional expression: An expression in a conditional jump always evaluates to either true or false no matter the input

Pointers

• [E:REFPASS] Passing an expired reference: eg. returning a ref. to a local ; passing a reference to a freed memory bloc
• [E:DEREF] Derefencing/passing an uninitialized pointer: Either dereferencing with the * or -> operator or passing the pointer to an API outside our control
• indexing error: (chose the sub-category that applies best to each case)
  • [E:OFFSET] Bad offset: eg. off-by-one, off-by-(i times constant), bad pointer arithmetic
  • [E:OVERFLOW] Buffer overflow: Like the bad offset but the offset is bound to end up outside the buffer and cause memory corruption/segfault

APIs

This section describes errors related to the usage of external APIs, whether part of stdlib or another lib used in the project.

• bad argument:
  • [E:ARG_E] causes or may cause a compile or runtime error: (eg. segfault, casting int to pointer). For uninitialized pointer, use passing uninitialized value instead.
  • [E:ARG_W] causes a wrong result
  • [E:ULESS_ARG] useless/overkill but not wrong: sometimes there is something odd with the way the student calls a function but it is not actually wrong.
• return value:
  • [E:RET_IGN] ignored: A non-void function's return value is ignored but it may carry critical information about that function execution (errors, number of elements treated, ...)
  • [E:RET_USG] poorly used (likely a bug): The return value is used but in a way that doesn't comply with the manpage/documentation.

Input (args, FILEs, stdin, network or APIs, ...)

• [E:NULLCHECK] missing null check
• sanitization: getting and cleaning up input data
  • [E:SAN_M] missing but without practical consequences (eg. '0' is supposed to return an error but instead the program runs '0' iterations and exits normally)
  • [E:SAN_MV] missing and causing security vulnerability (eg. bad scanf or some input data may cause a crash or a memory corruption)
  • [E:SAN_W] wrong but without practical consequences
  • [E:SAN_WV] wrong and causing a security vulnerability
• [E:IFORMAT] bad handling of the input format: the input format is incorrectly read, except if this qualifies as a missing edge case then use missing edge case.

Output (stdout, status code, FILEs, network or APIs like fuse, ...)

• [E:OFORMAT] bad output format: (eg. printf("%d", i) instead of printf("- %d", i); however, this would be a bad argument: printf("- %d", (unsigned) i);)

Error handling

• [E:ERRCHECK] missing error check
• [E:ERRCODE] wrong error code
• [E:ERR_NOERR] reporting an error which is not an error

Memory management

• [E:ULESS_ALLOC] useless malloc: A local variable could have been used instead
• [E:MISS_FREE] missing free
• [E:DBL_FREE] double free
• [E:MUCH_ALLOC] allocating too much memory
• [E:FEW_ALLOC] allocating too few memory
• [E:UNDEF_ALLOC] allocating a non deterministic amount of memory

Types

• implicit cast (chose the one that best applies)
  • [E:IMPL_CAST] no consequences
  • [E:CONST] discarding const qualifier
  • [E:SEM_CAST] semantic error: The program may run correctly but the semantics have been wronged (eg. signed vs unsigned, losing part of the meaning of the variable without a proper check)
  • [E:BAD_CAST] suspected or confirmed bug
• [E:PROTO_W] prototype mismatch (warning)
• [E:PROTO_E] prototype mismatch (error)
• [E:LOCAL] bad type for a local variable
• [E:MEMBER] bad type for a member
• [E:ULESS_MEMBER] useless member

Examples

This section shows hypothetical code snippets along with the grading comments left by the TA.

Example 1:

The context sheet (given by the teacher):

[YPAGES]: fonction read_ypages
[GLOBAL]: le reste

The student's code:

int read_ypages(ypages_t* yp, const char* const fname)
{
    // correcteur: [E:YPAGES:NULLCHECK] In this instance we can not safely assume yp and fname are not null

    FILE* in = NULL;
    in = fopen(fname, "r");
    // correcteur: [E:YPAGES:RET_USG] Vérifier le retour de fopen
    
    fscanf(in, "%u", &yp->NEnt); /*We get the number of contacts*/

    /*We allocate the memory*/
    yp->contacts = calloc(yp->NEnt, sizeof(contact_t));
    yp->indices = calloc(yp->NEnt, sizeof(contact_t*));

    // correcteur: [E:YPAGES:RET_USG] si yp->NEnt vaut 0 alors calloc peut retourner NULL même s'il n'y a pas eu d'erreur
    if(yp->contacts == NULL || yp->indices == NULL) {
        fprintf(stderr, "[ERROR]: Cannot allocate the memory (code %d)\n", ERR_IO);
        return ERR_MALLOC;
    }

    for(int i = 0; i < yp->NEnt; i++) {
        yp->indices[i] = &yp->contacts[i];
        
        // correcteur: [E:YPAGES:SAN_MV] fscanf ne protège pas contre les dépassements de tampon
        // correcteur: [E:YPAGES:RET_IGN] il faut vérifier la valeur de retour de fscanf
        fscanf(in, "%s", yp->indices[i]->name.family);

        // correcteur: [E:YPAGES:SAN_MV] [E:YPAGES:RET_IGN]
        fscanf(in, "%s", yp->indices[i]->name.given);

        // note: le tag est placé une fois par occurence de l'erreur
        // correcteur: [E:YPAGES:SAN_MV] [E:YPAGES:RET_IGN]
        fscanf(in, "%hu %hu %lu", &yp->indices[i]->phone.country_code, &yp->indices[i]->phone.area_code, &yp->indices[i]->phone.number);
    }
    fclose(in);
    return 0;
}

Example 2:

The context sheet (given by the teacher):

[ORDER] struct order_t

[RESERV] struct reserv_t (sauf liste chainée)
[RESERV_LC] partie "liste chainée" de struct reserv_t

[COACH] struct coach_t (sauf liste chainée)
[COACH_LC] partie "liste chainée" de struct coach_t

[TRAIN] struct train_t

[GLOBAL]: le reste

The student's code:

typedef struct {
    unsigned int id;
    unsigned int ntickets;
    double weight;
} order_t;

typedef struct reserv_t_ {
    order_t* order;
    struct reserv_t_ *next;

    // correcteur: [E:RESERV_LC:ULESS_MEMBER] Une liste simplement chainée aurait suffit
    struct reserv_t_ *previous;
} reserv_t;

typedef struct {
    // correcteur: [E:COACH_LC:MEMBER] Ceci n'est pas une liste chainée
    unsigned int free_seats;
    double weight;
    reserv_t* reservs;
} coach_t;

typedef struct {
    unsigned int npassengers;
    unsigned int ncoaches;
    unsigned int max_coaches;

    // Ici on ne pénalise pas le type ** bien que la solution utilise un * car le reste de
    // l'implémentation est cohérent avec cette structure. La pénalité pour la liste chainée
    // a déjà été infligée dans le critère lui correspondant
    coach_t** coaches;
} train_t;