Augusto Dias Noronha
Augusto Dias Noronha

Reputation: 864

Output changes in every run reading from file

I'm writing a small application for college that parses some wikipedia pages and outputs information about the people in the pages.

I wrote it in Java and am trying to re-write it in C. I'm getting a weird bug, sometimes the program's output is correct and sometimes it is wrong, without changing the input.

Here is a sample input that triggers the error with the name "105.html"

This is the output I get sometimes:

105 Linus Pauling Estadunidense 28 de fevereiro de 1901 Portland, Oregon 19 de agosto de 1994 Big Sur, Califórnia 93

This is the output I get other times:

105 Linus Pauling Estadunidense 28 de f@evereir@o�y� dC�L��e ���y�19I�L��01 Portland, Oregon 19 de agosto de 1994 Big Sur, Califórnia 93

I notice that if I set a breakpoint in XCode, I usually get the RIGHT result...

I'm new to C so I actually have no clue how to start debugging this.

Here is the code if anyone is interested in actually reading it. The code is in a mixture of Portuguese and English but I added English comments so it should be easy to follow.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct pessoa{
    int id;
    char *nome; //name
    char *nacionalidade; // nationality
    char *nascimento; // date of birth
    char *local_nascimento; // place of birth
    char *morte; // date of death
    char *local_morte; // place of death
    int idade; // age
};


struct pessoa *inicializar(int n) {
    struct pessoa *pessoas = malloc(sizeof(pessoas) * n);
    return pessoas;
}

void imprimir_pessoa(struct pessoa *p) {
    printf("%i %s %s %s %s %s %s  %i\n", p->id, p->nome, p->nacionalidade,
           p->nascimento, p->local_nascimento, p->morte,
           p->local_morte, p->idade);
}

void imprimir_pessoa_asterisco(struct pessoa *p) {
    printf("%i ## %s ## %s ## %s ## %s ## %s ## %s ## %i\n", p->id, p->nome, p->nacionalidade,
           p->nascimento, p->local_nascimento, p->morte,
           p->local_morte, p->idade);
}

size_t index_of(char *string, char *to_find) {
    return strstr(string, to_find) - string;
}

char *remove_tags(char *string) {
    // inicializa para o mesmo tamanho da string de entrada para garantir que ira caber
    char * resp = malloc(sizeof(char) * strlen(string) + 1);

    // jumps over the html tags and finds the aproppriate information
    for (size_t i = 0; i < strlen(string); i++) {

        while (i < strlen(string) && string[i] == '<') {
            for (i++; string[i] != '>'; i++);
            i++;

            while(i < strlen(string) && string[i] == '&'){
                for (i++; string[i] != ';'; i++);
                i++;
            }
        }

        while(i < strlen(string) && string[i] == '&'){
            for (i++; string[i] != ';'; i++);
            i++;
            resp[strlen(resp)] = ' ';
        }


        if (i < strlen(string)) {
            resp[strlen(resp)] = string[i];
        }
    }

    while(strlen(string) > 0 && resp[0] == ' '){ // jumps over white spaces on the begining
        resp += 1;
    }
    resp[strlen(resp)] = 0;

    return resp;
}

char* extrair_nome(char *string) { // extract the person's name
    size_t index = index_of(string, "<title>") + strlen("<title>");
    size_t index_fim = index_of(string, " Wiki") - 4;
    char *nome = malloc(sizeof(char) * (index_fim - index));
    memcpy(nome, (string+index), index_fim - index);
    return nome;
}

char* substring(char * string, char *c) {
    return string + strcspn(string, c);
}

void remove_new_line(char *string) {
    char *pos;
    if ((pos=strchr(string, '\n')) != NULL)
        *pos = '\0';
}

void ler_pessoa(char *nome_arquivo, struct pessoa *p) { // parse the file to fill the pessoa struct
    size_t length = strlen(nome_arquivo);
    p->id = (nome_arquivo[length - 8] - 48) * 100;
    p->id = (p->id + (nome_arquivo[length - 7] - 48) * 10);
    p->id = p->id + (nome_arquivo[length - 6] - 48);

    int tamanho_linha = 2000;
    char *linha = malloc(sizeof(char) * tamanho_linha);
    FILE *fp = fopen(nome_arquivo, "r");

    if (fp == NULL) {
        printf("Falha ao abrir arquivo %s\n", nome_arquivo);
        exit(1);
    }

    while (fgets(linha, tamanho_linha, fp) != NULL) {
        if (strstr(linha, "<title>")) { // extracts name
            p->nome = extrair_nome(linha);
            remove_new_line(p->nome);
            break;
        }
    }

    while (fgets(linha, tamanho_linha, fp) != NULL) {
        if (strstr(linha, "Nacionalidade")) { // extracts nationality
            fgets(linha, tamanho_linha, fp);
            p->nacionalidade = remove_tags(linha);
            remove_new_line(p->nacionalidade);
            break;
        }
    }

    while (fgets(linha, tamanho_linha, fp) != NULL) {
        if (strstr(linha, "Nascimento")) { // extracts date of births
            fgets(linha, tamanho_linha, fp);
            p->nascimento = remove_tags(linha); // <-- this one is not working all the time??
            remove_new_line(p->nascimento);
            break;
        }
    }

    //se vivo
    if (strstr(p->nascimento, ")") != NULL) { // if the person is alive the date of birth date is of the type: date(age)
        char *tmp = malloc(sizeof(char) * strlen(p->nascimento)); // so we extract the age
        strcpy(tmp, p->nascimento);
        tmp = tmp + strcspn(tmp, "(") + 1;
        tmp[index_of(tmp, " ")] = 0;
        p->idade = atoi(tmp);
        p->morte = "vivo"; // not dead
        p->local_morte = "vivo"; // not dead
    } else {
        p->morte = ""; // we set this later
        p->local_morte = "";
    }

    while (fgets(linha, tamanho_linha, fp) != NULL) {
        if (strstr(linha, "Local")) { // extracts place of death
            fgets(linha, tamanho_linha, fp);
            p->local_nascimento = remove_tags(linha);
            remove_new_line(p->local_nascimento);
            break;
        }
    }

    if (strlen(p->morte) == 0) { // we set this now if the person is not alive (hence size 0)
        while (fgets(linha, tamanho_linha, fp) != NULL) {
            if (strstr(linha, "Morte")) { // extract death day
                fgets(linha, tamanho_linha, fp);
                p->morte = remove_tags(linha);
                remove_new_line(p->morte);
                break;
            }
        }

        if (strstr(p->morte, "(") != NULL) {
            char *tmp = malloc(sizeof(char) * strlen(p->morte));
            strcpy(tmp, p->morte); // extract age when the person died, like above
            tmp = tmp + strcspn(tmp, "(") + 1;
            tmp[index_of(tmp, " ")] = 0;
            p->idade = atoi(tmp);
            p->morte[index_of(p->morte, "(")] = 0;
        }

        while (fgets(linha, tamanho_linha, fp) != NULL) {
            if (strstr(linha, "Local")) { // get the place of death
                fgets(linha, tamanho_linha, fp);
                p->local_morte = remove_tags(linha);
                remove_new_line(p->local_morte);
                break;
            }
        }
    }

    fclose(fp);
}


int main(int argc, const char * argv[]) {
    struct pessoa p;
    ler_pessoa("/tmp/105.html", &p);
    imprimir_pessoa(&p);
    return 0;
}

Upvotes: 1

Views: 54

Answers (1)

chux
chux

Reputation: 153498

resp[strlen(resp)] = ' '; and resp[strlen(resp)] = string[i]; are bad as resp[] is not certainly null character terminated.

Code needs a new approach to determine which element of resp[] to assign.


resp[strlen(resp)] = 0; is questionable too.

strlen(resp) returns the length of the string, not counting the null terminator. For strlen() to work well, resp must be null terminated first, else it is not referencing a string. The null character is in the index that equals the length, so resp[strlen(resp)] = 0; is a no-op function, other than killing some CPU cycles.


Code has other problems.
Example: Insufficient space. @Weather Vane;

// bad code
char *tmp = malloc(sizeof(char) * strlen(p->nascimento)); // so we extract the age
strcpy(tmp, p->nascimento);

Sample string allocator/duplicator (Note: strdup() often exists on many platforms)

char *strdupe(const char *s) {
  size_t size = strlen(s) + 1;
  dupe = malloc(size);
  if (dupe) {
    memcpy(dupe, s, size);
  }
  return dupe;
}

Upvotes: 1

Related Questions