PI-ITBA / 2024_02

Consultas 2C 2024
4 stars 0 forks source link

Ejercicio Taller BIBLE #154

Open juanhumphreys opened 3 weeks ago

juanhumphreys commented 3 weeks ago

Hola me tira segmentation fault y no entiendo por que .

#include <assert.h>
#include <string.h>
#include <stdlib.h>
#include "bibliaADT.h"
#define DIMbooks 76
#define MAX_VERSES 2146
#define BLOQUE 10

typedef char ** tverses;
typedef struct books * Tbook;

char * cpyverse(const char * verse);

struct bibleCDT{
    Tbook books[DIMbooks];  
};

struct books{
    tverses versiculos;  
    int dim; 
};

bibleADT newBible(){
    bibleADT aux = calloc(1, sizeof(struct bibleCDT));
    for(int i = 0 ; i< DIMbooks; i++){
        aux->books[i] = NULL;
        aux->books[i]->dim =0;

    }
    return aux;
}

int addVerse(bibleADT bible, size_t bookNbr, size_t verseNbr, const char * verse){
    if(bible != NULL || bookNbr > DIMbooks ||verseNbr > MAX_VERSES || bible->books[bookNbr-1]->versiculos[verseNbr-1] != NULL){
        return 0;
    }

   if(verseNbr > bible->books[bookNbr-1]->dim){
       bible->books[bookNbr-1]->versiculos[verseNbr-1] = malloc(1 * sizeof(tverses));
       bible->books[bookNbr-1]->versiculos[verseNbr-1] = cpyverse(verse);
        return 1;
    }

    bible->books[bookNbr-1]->versiculos[verseNbr-1] = cpyverse(verse);
    bible->books[bookNbr-1]->dim++;
    return 1;
}

char * cpyverse(const char * verse){
    char * aux;
    int i;

    for(i =0; verse[i] != 0; i++){
      if(i == BLOQUE-1){
        aux = malloc(BLOQUE * sizeof(char *));
      }
        aux[i] = *verse;
        aux++;
        verse++;
    }
    aux[i] = 0;
    return aux;
}

char * getVerse(bibleADT bible, size_t bookNbr, size_t verseNbr){
    if( bookNbr > DIMbooks || verseNbr > bible->books[bookNbr-1]->dim || bible->books[bookNbr-1]->versiculos[verseNbr-1] == NULL){
        return NULL;
    }
    char * aux = bible->books[bookNbr-1]->versiculos[verseNbr-1];
    return aux;
}

void freeBible(bibleADT bible){
    if(bible== NULL){
        return;
    }

    for(int i = 0; i < DIMbooks; i++){
        for(int j = 0; j < bible->books[i]->dim; j++){
            free(bible->books[i]->versiculos[j]);
        }
       free(bible->books[i]);
    }

    free(bible);
    return;
}
tomaspietravallo commented 3 weeks ago

No estoy con la compu a mano para correrlo, pero veo dos cosas:

  1. No podes asumir que existe la constante MAXVERSES, ya que eso te evita usar memoria dinámica para el array de versos y simplifica el ejercicio
  2. Si bien asumiste que existía la constante, no la usaste, por qué nunca alocaste el espacio para el arreglo de versos — igualmente esto tendría que alocarse de manera dinámica a medida que se agregan los versos

Quizás por esto te esté dando seg fault, si me incluis el mensaje de error completo lo miro, si no en un rato corro el código.

Saludos Juani!

tomaspietravallo commented 3 weeks ago

Otra cosita que veo es las líneas:

aux->books[i] = NULL;
aux->books[i]->dim =0;

cuando hagas books[i]->dim estas desreferenciando NULL (ya que dijiste que books[i] es NULL en la línea anterior)

juanhumphreys commented 3 weeks ago

Una pregunta el alloc del vector de verse cuando lo deberia hacer, yo pense que lo estaba haciendo cuando hacia el addverse, otra cosa tenes razon con lo del books[i] apuntando al null, me parece que cuando hago el addverse lo estoy desreferenciando denuevo y que de ahi sale el seg fault. por lo tanto estaria bien hecha la funcion newbible entonces (obviamente sacando lo de dim) y acmbio el adverse? o cambio el newbible para que funcione correctamente el resto.

tomaspietravallo commented 3 weeks ago

En newBible estas haciendo calloc entonces no hace falta que inicialices las cosas en cero (viene gratis con el calloc), ya que los libros son elementos estáticos de bibleCDT, y no punteros a zonas dinámicas sin inicializar.

Te agrego unos comentarios a la función addVerse que capaz así se entiende mejor:

int addVerse(bibleADT bible, size_t bookNbr, size_t verseNbr, const char * verse){
// esto esta bien salvo por MAX_VERSES
    if(bible != NULL || bookNbr > DIMbooks ||verseNbr > MAX_VERSES || bible->books[bookNbr-1]->versiculos[verseNbr-1] != NULL){
        return 0;
    }

   // aca estas preguntando si el versículo a agregar es mayor que la cantidad de espacio que tener para guardar los versículos (dim) 👍
   // pero lo que deberías hacer es alocar mas memoria para poder guardar una cantidad mayor de versiculos
   // si bible->books[bookNbr-1]->versiculos es el puntero a versiculos y no tiene suficiente espacio, deberías realocarlo (y modificar dim)
   if(verseNbr > bible->books[bookNbr-1]->dim){
       // acá estas haciendo un malloc pero después pisas ese valor en la siguiente linea (memory leak)
       bible->books[bookNbr-1]->versiculos[verseNbr-1] = malloc(1 * sizeof(tverses));
       bible->books[bookNbr-1]->versiculos[verseNbr-1] = cpyverse(verse);
        return 1;
    }

    // nota: cpyverse esta mal, debería realocar a medida que necesita espacio, no hacer varios malloc
    bible->books[bookNbr-1]->versiculos[verseNbr-1] = cpyverse(verse);
    // si ya tenias espacio suficiente para guardarlo entonces la dimension del vector no cambia 
    bible->books[bookNbr-1]->dim++;
    return 1;
}
juanhumphreys commented 3 weeks ago

ahi me salio muchas gracias!!!!!!!