Exam exercise about lists and pointers C

Question

i need to fix this exam exercise since my teacher will ask me how to fix this tomorrow at oral test:

nodo *CancellaTutto(nodo *a, char *k) {

nodo *p,*q;

p = a;

if (p == NULL)
    return NULL;

while (p != NULL) {

    if (strcmp(p->chiave, k) == 0 ) {


        if (p->prec == NULL && p->succ == NULL)
            return NULL;

        if (p->succ == NULL && p->prec != NULL) {
            q = p;

            p = p->prec;
            p->succ = NULL;
            free(q);
        }

        if (p->prec == NULL && p->succ != NULL) {
            q = p;

            p = p->succ;
            p->prec = NULL;
            free(q);
        }

        if (p->prec != NULL && p->succ != NULL) {
            q = p;

            p = p->succ;
            q->prec->succ = p;
            p->prec = q->prec;
            free(q);
        }

    } else { p = p->succ; }

}

return a;
}

This function should see if two string are equals (one in a struct linked list and the other is the k string) and erase all string equal to k, but there are two output cases that are obviously wrong:

CASE 1:

k is : DOG

if i insert 3 strings in a : DOG -> CAT -> CAT the function doesnt erase "DOG" and show me output: DOG -> CAT -> CAT (correct output is CAT -> CAT)

CASE 2:

Another error i found is: if a list is : DOG -> DOG -> CAT i get output DOG -> DOG -> CAT (right output should be: CAT)

All other cases should work right.

struct is :

struct nodo {
char *chiave;
struct nodo *prec;
struct nodo *succ;
};
typedef struct nodo nodo;

rest of code is: (Read only to comprehend this part is just for a personal test; useless for the exam)

int main()
{
nodo *lista=CreateListString(); // create a list
Visualizza(lista); // views it
char *stringa="ciao"; // create k string
lista=CancellaTutto(lista,stringa); // call function
Visualizza(lista); // views it
}

Please note that I need just to fix this, not to write another code.

Please dont look at overflows, errors, and such things in these function! Just fix the first function! others are for a personal test.

a note: `sizeof(char)` is always `1` in C, it's redundant usage here. — Sourav Ghosh, Jun 29 '15 at 14:29
Also common warning: [Do not cast the return of malloc.](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Eregrith, Jun 29 '15 at 14:30
@SouravGhosh That is incorrect mentality about sizeof.. If you ever read someone else's code then you would recognize that programmer's intent and the idea of self documenting code. This is also the same reason magic numbers are harmful. — this, Jun 29 '15 at 14:32
@this Is the intent here to produce a buffer overflow vulnerability? — Eregrith, Jun 29 '15 at 14:33
@this please explain, how adding `sizeof(char)` adds readability? if it is `sizeof *(p->chiave)`, I can agree, but how with `char`. Rather, it limits the flexibility, IMHO. — Sourav Ghosh, Jun 29 '15 at 14:35
I'm going to take Sourav's advice one step further - never use a type name as a `sizeof` argument in a `malloc` call, whether it's `char`, `int`, or `struct somereallyhugestructtype`.. Instead, use something like `p = malloc( N * sizeof *p);` - this will always do the right thing regardless of the type of `p` (as long as it's a pointer type, anyway). — John Bode, Jun 29 '15 at 14:41
I agree with @JohnBode. Any approach using explicit type names is bad practice. What if the type of the variable changed? This is even worse for standard types, as they cannot be internally modified like a struct. There is little gained using `sizeof(char)`. — too honest for this site, Jun 29 '15 at 14:47
@this and @SouravGhosh : It should be `sizeof(*(p->chiave))` indeed because not only does it show what you're doing, it also prevents errors when/if changing `p->chiave`'s type — Eregrith, Jun 29 '15 at 14:49

score 0 · Answer 1 · answered Jun 29 '15 at 18:02

@BLUEPIXY has given a really strong hint in the comments, which I am going to expand on:

CASE 1:

k is : DOG

if i insert 3 strings in a : DOG -> CAT -> CAT the function doesnt erase "DOG" and show me output: DOG -> CAT -> CAT (correct output is CAT -> CAT)

CASE 2:

Another error i found is: if a list is : DOG -> DOG -> CAT i get output DOG -> DOG -> CAT (right output should be: CAT)

In both of these cases, the string you want to remove occurs at the head of the list. CancellaTutto needs to return the new head of the list in this case, which will not be a¹

^{1. If a is supposed to represent the head of your list, please use a more meaningful name for the variable, such as head or listHead (or whatever your native language equivalent would be).}

The issue is that i need to ERASE the string equal to k in the list 'a' (In this case, DOG) so i need to return a modified list 'a' not just to print a section of a. — Michele Bottini, Jun 29 '15 at 18:27

Exam exercise about lists and pointers C

1 Answers1