Sebastião
Sebastião

Reputation: 150

memset clearing what it's not supposed

I have a server - client program and which I can add or remove users. When the maximum user number is achieved (10), I have a bug when deleting one of the user.

Here is a piece of the function that is deleting the user:

int i;
int confirmacao = 0;
msg_t msg;
char cliname[10];
if (strlen(estrutura.id) == 3) {
    for (i = 0; memoria->x[i].id[0] != '\0'; ++i) {
        if (strcmp(estrutura.id, memoria->x[i].id) == 0) {
            confirmacao = 1;
            for (; memoria->x[i].id[0] != '\0'; ++i) {
                memoria->x[i] = memoria->x[i + 1];
            }
            // I think the problem is with this memset
            memset(memoria->x[i].id, '\0', sizeof(memoria->x[i].id));
            printf("IA antes de decrementar: %i\n", memoria->ia);
            memoria->ia--;
            printf("Ia depois: %i\n", memoria->ia);
            break;
        }
    }
} else {
    ...
}

Here is the print from gdb

memoria->ia is suposed to be decremented by one and I can figure out why memset is setting it to zero.

memoria is a global pointer for this structure:

typedef struct mmap_uti_s {
    uti_t x[NUTI];
    int ia;
} mmap_uti_t;

and uti_t is this structure:

typedef struct uti_s {
    char id[NDIG + 1];
    char nome[NDIM + 1];
    char portas[NPOR + 1];
} uti_t;

Upvotes: 1

Views: 89

Answers (1)

chqrlie
chqrlie

Reputation: 144951

You fail to reset the last element after copying of the previous one because you increment i before testing for the end of the array. Furthermore, as you correctly noted, you must always stop the copying before you reach the end of the array.

You should change the test in the nested for loop to:

for (; i + 1 < 10 && memoria->x[i + 1].id[0] != '\0'; i++)

And you should also change the test in the main loop to stop the scan at the end of the array if it is full.

Although not strictly necessary, it would be better style to use a different index for the copying phase, to avoid modifying the index of the current loop.

Clearing the whole structure, not just the id member, seems preferable too.

Here is the modified code:

int i, j;
int confirmacao = 0;
msg_t msg;
char cliname[10];
if (strlen(estrutura.id) == 3) {
    for (i = 0; i < 10 && memoria->x[i].id[0] != '\0'; ++i) {
        if (strcmp(estrutura.id, memoria->x[i].id) == 0) {
            confirmacao = 1;
            for (j = i; j + 1 < 10 && memoria->x[j].id[0] != '\0'; ++j) {
                memoria->x[j] = memoria->x[j + 1];
            }
            // I think the problem is with this memset
            memset(&memoria->x[j], 0, sizeof(memoria->x[j]));
            printf("IA antes de decrementar: %i\n", memoria->ia);
            memoria->ia--;
            printf("Ia depois: %i\n", memoria->ia);
            break;
        }
    }
} else {
    ...
}

Upvotes: 1

Related Questions