[NSWI004] Comments for A00 (printf and lists)

[Vojtech Horky]

Hello,

I will be uploading comments for A00 assignment to your grading 
repository shortly.

This assignment was not graded (please, do not worry about the 0 
points); we provide feedback as comments of your source code.


Generally, the solutions were rather nice and most of you submitted the 
extended assignment too.


A lot of the comments are related to style. Do not take them as strict 
orders how to do things but rather as thoughts on how to do things 
differently (and perhaps better in the long term).

The keyword that was inherent to many solutions was *consistency*. Be 
consistent in how you name things, how you format your code and avoid 
surprises. Such surprises include hiding statements on the same line as 
the "if" that are followed by badly indented line or functions that have 
side effects one would not expect.


Basically only one type of functional error was present in more 
solutions - insufficient handling of dynamic memory allocations. Follows 
a short description of each issue.

We acknowledge that handling these issues below is not trivial. The C 
language does not have exceptions, destructors etc. and one has to be 
extra careful. But don't be scared :-)

Here are the three typical issues.

(1) Not checking that `malloc` returned a valid memory (i.e. non NULL). 
This is something unheard of in languages with managed memory and you 
would typically not encounter such situations in C anyway. Unless you 
move into kernel. You will see that running out of memory is quite a 
normal situation and you should prepare your code for that.

Note that proper response is to return NULL or ENOMEM or similar, not to 
halt your application. Allow the caller to decide what to do.

(2) Leaking memory. Either by not freeing all items of a list or in more 
subtle situations like the one below.

void *ptr1 = malloc(..);
if (ptr1 == NULL) { return NULL; }
void *ptr2 = malloc(...);
if (ptr2 == NULL) { return NULL; }

This code leaks memory of ptr1 if the second allocation fails. Correct 
approach is to add free(ptr1) the second "if" or use error gotos as are 
for example used in Linux [1].

[1] 
https://github.com/torvalds/linux/blob/156c05917e0920ef5643eb54c0ea71aae5d60c3d/tools/testing/selftests/cgroup/test_memcontrol.c#L27

(3) Memory corruption by using memory after freeing it. Following 
pseudo-code for linked list deallocation is not correct although it will 
probably run without any issues in many environments.

while (it != NULL) {
    free(it);
    it = it->next;
}

Please not that (2) is not something inherent to C. It would not happen 
in managed languages with memory but it can easily happen with other 
resources. Such as opened files, network connections or database cursors.


If you have any questions regarding the comments, please, e-mail us. 
Preferably off-list, we can forward to the list explanations that were 
not unique to your implementation.

Cheers,
- VH