Why I get wrong values in dynamic memory?

Question

I'm writing a program that store a list of topics with linked structs. The program read a list of topics from a file and create a first topic "metatema", and then, the topics from the file. When I write 4 or 5 topics in the file, it works fine. But when I write more topics and I print the values, I get incorrect values.

Here is the relevant code of my program:

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <arpa/inet.h>
#include <fcntl.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <ctype.h>

#define BUFFER_SIZE 256


/*   STRUCTS   */
typedef struct user {
    struct  sockaddr * direccion;   /* Dir from user */
    struct  user *  siguiente;      /* Next user node */
};

typedef struct topic {
    char *      nombre;         /* Topic name */
    int numero_usuarios;        /* Number of users */
    struct user *   lista;      /* Pointer to the users list */
    struct topic *  siguiente;  /* Next topic */
};

/* GLOBAL VARs */
int numero_temas = 0;
struct topic * temas;
int tam_topic = 0;
int tam_user = 0;

/* FUNCTIONS */
void leerTemas(char * file);

int main(int argc, char *argv[]) {
    if (argc!=3) {
        fprintf(stderr, "Use: %s port file\n", argv[0]);
        return 1;
    }

    leerTemas(argv[2]);

    /* PRINT VALUES */
    int i=0;
    for(i=0; i<numero_temas;i++){
        printf("TOPIC: %s\tADDRESS: %u",(temas + i*sizeof(struct topic))->nombre,(temas + i*sizeof(struct topic)));
        printf("\tNEXT: %u\tUSERS LIST: %u\t NUM. USERS: %u\n",(temas + i*sizeof(struct topic))->siguiente,
            (temas + i*sizeof(struct topic))->lista, (temas + i*sizeof(struct topic))->numero_usuarios);
    }
    return 0;
}

void leerTemas(char * file){
    int pipefd[2];
    if(pipe(pipefd) == -1){
        perror("pipe");
        exit(1);
    }

    /* CALCULAMOS EL NUMERO DE TEMAS PARA RESERVAR MEMORIA */
    numero_temas = 0;

    switch(fork()){
        case -1:                    // ERROR
            perror("fork");
            exit(1);
        case 0:                     // CHILD (WC)
            close(pipefd[0]);
            close(1);
            dup(pipefd[1]);
            close(pipefd[1]);
            execlp("wc","wc",file,(char *)NULL);
            perror("exec");
            exit(1);
        default:                    // PARENT (GET RESULT FROM WC)
            close(pipefd[1]);
            char buffer[BUFFER_SIZE];   bzero(buffer,BUFFER_SIZE);
            char ntemas[BUFFER_SIZE];   bzero(ntemas,BUFFER_SIZE);
            if(read(pipefd[0],buffer,BUFFER_SIZE) <= 0){
                perror("read");
                exit(1);
            }

            /* GETTING THE NUMBER OF LINES FROM WC */
            int i=0,j=0;
            while(buffer[i] == ' ')         // Pass blank spaces
                i++;
            while(buffer[i] != ' '){
                ntemas[j] = buffer[i];
                i++;
                j++;
            }
            ntemas[j] = '\0';
            numero_temas = atoi(ntemas)+1;  // +1 (metatema)
    }

    /* ALLOCATING TOPICs MEMORY */
    temas = malloc(sizeof(struct topic) * numero_temas);
    bzero(temas,sizeof(struct topic)*numero_temas);

    /* CREATING METATEMA */
    temas->nombre = malloc(sizeof(char)*9);
    strcpy(temas->nombre,"metatema\0");
    temas->siguiente = NULL;
    temas->lista = NULL;
    temas->numero_usuarios = 0;

    /* CREATING TOPICs FROM FILE */
    FILE * f = fopen(file,"r");
    int i;
    char buffer[BUFFER_SIZE];
    for(i = 1; i < numero_temas; i++){
        bzero(buffer,BUFFER_SIZE);
        fgets(buffer,BUFFER_SIZE,f);
        buffer[strlen(buffer)-1] = '\0';    // Remove newline char \n

        (temas + i*sizeof(struct topic))->nombre = malloc(sizeof(char)*(strlen(buffer)+1));
        strcpy((temas+i*sizeof(struct topic))->nombre,buffer);
        (temas + i*sizeof(struct topic))->numero_usuarios = 0;
        (temas + i*sizeof(struct topic))->lista = NULL;
        (temas + i*sizeof(struct topic))->siguiente = NULL;
        (temas + (i-1)*sizeof(struct topic))->siguiente = (temas + i*sizeof(struct topic)); // Link prev topic
    }
}

When I run the program with 5 topics in the file run fine:

$ cat topics
topic1
topic2
topic3
topic4
topic5
$ ./a.out 8000 topics
TOPIC: metatema ADDRESS: 31891472   NEXT: 31892496  USERS LIST: 0    NUM. USERS: 0
TOPIC: topic1   ADDRESS: 31892496   NEXT: 31893520  USERS LIST: 0    NUM. USERS: 0
TOPIC: topic2   ADDRESS: 31893520   NEXT: 31894544  USERS LIST: 0    NUM. USERS: 0
TOPIC: topic3   ADDRESS: 31894544   NEXT: 31895568  USERS LIST: 0    NUM. USERS: 0
TOPIC: topic4   ADDRESS: 31895568   NEXT: 31896592  USERS LIST: 0    NUM. USERS: 0
TOPIC: topic5   ADDRESS: 31896592   NEXT: 0 USERS LIST: 0    NUM. USERS: 0

But, when I set 7 topics (for example), I get wrong values:

$ cat topics
topic1
topic2
topic3
topic4
topic5
topic6
topic7
$ ./a.out 8000 topics
TOPIC: metatema ADDRESS: 7274512    NEXT: 7275536   USERS LIST: 0    NUM. USERS: 0
TOPIC: topic1   ADDRESS: 7275536    NEXT: 7276560   USERS LIST: 1768976244   NUM. USERS: 33
TOPIC: topic2   ADDRESS: 7276560    NEXT: 7277584   USERS LIST: 0    NUM. USERS: 0
TOPIC: topic3   ADDRESS: 7277584    NEXT: 7278608   USERS LIST: 0    NUM. USERS: 0
TOPIC: topic4   ADDRESS: 7278608    NEXT: 7279632   USERS LIST: 0    NUM. USERS: 0
TOPIC: topic5   ADDRESS: 7279632    NEXT: 7280656   USERS LIST: 0    NUM. USERS: 0
TOPIC: topic6   ADDRESS: 7280656    NEXT: 7281680   USERS LIST: 0    NUM. USERS: 0
TOPIC: topic7   ADDRESS: 7281680    NEXT: 0 USERS LIST: 0    NUM. USERS: 0

And, when I set 11 topics, I get this:

$cat topics
topic1
topic2
topic3
topic4
topic5
topic6
topic7
topic8
topic9
topic0
topicA
$ ./a.out 8000 topics
a.out: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.
Abortado

What am I doing wrong?

Answer 1

This is the problem

(temas + i*sizeof(struct topic))

the compiler automatically computes the offset for you, i.e.

(temas + i)

is what you need, or

((char *)temas + i*sizeof(struct topic))

because this way the compiler will not multiply i by sizeof(struct topic), but in the previous case it will so your code actually means

((char *)temas + i*sizeof(struct topic)*sizeof(struct topic))

so you are adding a lot more than what you are supposed to, you don't need to use pointer arithmetic like this, just

temas[i]

would be sufficient.

Also, there is absolutely no need for global variables, just pass them as function parameters, global variables can cause a lot of problems, and you should not use them unless you are completely sure that you need to.