2010-02-02 5 views
2

Je fais mes dents depuis 48 heures en essayant d'implémenter cette fonction de table de hachage en C. Mon code est assez long (je me rends compte que ce n'est pas le plus efficace, certains sont plus moi jouant avec C pour avoir une idée de comment cela fonctionne, etc).Pointers d'apprentissage en C

Le problème que j'ai est avec la dernière ligne de mon programme principal en bas (impression MyEntry-> Name). Je reçois une erreur de bus et je ne sais pas pourquoi. Je ne crois pas que je suis censé allouer de la mémoire dans le pilote principal pour ce pointeur, mais je peux me tromper.

Désolé pour la longueur de ce code. BTW SymEntry est « SymEntry struct {char * Nom, attributs void *, struct SymEntry * Suivant}

#include <strings.h> 
#include <stdio.h> 
#include <stdlib.h> 
#include <ctype.h> 
#include <stdbool.h> 
#include "SymTab.h" 



struct SymTab * CreateSymTab(int Size) 
{ 
    struct SymTab *symtable; 
    if(!(symtable=malloc(sizeof(struct SymTab)))) return NULL; 
    if(!(symtable->Contents=calloc(Size, sizeof(struct SymEntry*)))) { 
      free(symtable); 
      return NULL; 
    } 

    symtable->Size=Size; 
    return symtable; 
} 

/* hash form hash value for string s, taken from 'The C Programming Language'*/ 
unsigned hash(struct SymTab *ATable, const char *s) 
{ 
    unsigned hashval, size; 
    size = ATable->Size;; 
    for (hashval = 0; *s != '\0'; s++) 
     hashval = *s + 31 * hashval; 
    return hashval % size; 
} 

bool EnterName(struct SymTab *ATable, 
      const char *Name, 
      struct SymEntry **AnEntry) 
{ 
      struct SymEntry *ptr; 
      unsigned hashvalue; 
      char *string; 
      struct SymEntry *previous; 

      string = malloc(strlen(Name)+1); 
      AnEntry=(struct SymEntry**)malloc(sizeof(struct SymEntry*)); 

      strcpy(string, Name); 
      printf("string is: is %s\n",string); 
      hashvalue = hash(ATable, string); 

      printf("hv is %d\n",hashvalue); 
      ptr = ATable->Contents[hashvalue]; 
      previous = NULL; 

      while(ptr) 
      { 
       printf("WHILE LOOP\n"); 
       if(!(strcmp(ptr->Name,string))) 
       { 
        printf("if(!strcmp(ptr->Name,string))\n"); 
        *AnEntry = ptr; 
        return true; 
       } 
       previous = ptr; 
       ptr=ptr->Next; 
      } 
      if(previous) 
      { 
       printf("IF (PREVIOUS)\n"); 
       if(!(ptr=malloc(sizeof(struct SymEntry)))) return false; 
       if(!(ptr->Name=string)) 
       { 
        printf("if(!(ptr->Name=string))\n"); 
        free(ptr); 
        return false; 
       } 
       ptr->Name = string; 
       previous->Next = ptr; 
       printf("Previous->Next: %s\n", previous->Next->Name); 
       *AnEntry = ptr; 
       return false; 
      } 
      else 
      { 
       printf("ELSE (PREVIOUS)\n"); 
       if(!(ptr=malloc(sizeof(struct SymEntry)))) return false; 
       if(!(ptr->Name=string)) 
       { 
        printf("if(!(ptr->Name=string))\n"); 
        free(ptr); 
        return false; 
       } 
       ptr->Name = string; 
       ATable->Contents[hashvalue] = ptr; 
       printf("here\n"); 
       *AnEntry = ptr; 
       printf("there\n"); 
       return false; 
      } 

} 

struct SymEntry * FindName(struct SymTab *ATable, const char *Name) 
{ 
    struct SymEntry *Entry; 
    unsigned hashvalue; 

    hashvalue = hash(ATable, Name); 
    Entry = ATable->Contents[hashvalue]; 

    while(Entry) 
    { 
       if(strcmp(Name,Entry->Name)==0) 
       { 
               return Entry; 
       } 
    } 
    return NULL; 
} 



main(int argc, char **argv) 
{ 
    struct SymTab *mysymtab; 
    struct SymEntry *myEntry; 

    mysymtab = CreateSymTab(1); 
    const char *string1 = "HELLO"; 
    printf("%d\n",6); 
    EnterName(mysymtab, string1, &myEntry); 
    printf("first: %s\n", mysymtab->Contents[0]->Name); 
    EnterName(mysymtab, string1, NULL); 
    EnterName(mysymtab, "WORLD", NULL); 
    printf("second: %s\n", mysymtab->Contents[0]->Name); 
    printf("second->Next: %s\n", mysymtab->Contents[0]->Next->Name); 
    EnterName(mysymtab, "[email protected]#$%", &myEntry); 
    printf("third: %s\n", mysymtab->Contents[0]->Name); 
    printf("third->Next: %s\n", mysymtab->Contents[0]->Next->Name); 
    printf("third->Next->Next: %s\n", mysymtab->Contents[0]->Next->Next->Name); 
    printf("myEntry->Name: %s\n", myEntry->Name); 
} 
+0

Si vous êtes nouveau C, et n'utilisez pas un débogueur (qui je suppose est la raison de tous les printfs) Je recommande de prendre le temps de pratique avec un comme il sauvera beaucoup d'heures et fera la viande hachée de beaucoup de bogues curieux. Telle a été mon expérience, de toute façon. –

Répondre

7

Le problème est cette ligne dans EnterName:

AnEntry=(struct SymEntry**)malloc(sizeof(struct SymEntry*)); 

Vous devez supprimer ce que vous voulez AnEntry pour pointer vers l'argument que l'appelant a spécifié.

Parce que AnEntry peut être NULL, vous aurez également besoin de changer tous les cas de:

*AnEntry = ptr; 

à:

if (AnEntry) 
    *AnEntry = ptr; 

Ce qui se passe est que lorsque la fonction démarre, AnEntry pointe vers le pointeur que l'appelant veut changer. Lorsque vous modifiez la valeur de AnEntry (par exemple, AnEntry = ...;), votre code ne modifiera pas le pointeur que vous souhaitez modifier, mais un pointeur interne. Par conséquent, lorsque EnterName renvoie, myEntry pointe toujours vers un emplacement aléatoire dans la mémoire.

+0

Merci qui a fonctionné. C'est logique maintenant que j'y pense. Merci encore! – James

+0

Alors que sur le sujet, chaque pointeur de SymEntry devrait être réglé sur NULL dans tous les cas, ce qui n'est pas le cas dans le cas 'else'. –

+0

ya j'ai attrapé ça aussi. En outre, il y a une boucle infinie dans la fonction FindName que j'ai corrigé maintenant ... j'ai oublié de parcourir la liste. – James

0

Pendant que vous apprenez, il y a des WTF stylistiques dans votre code. Prenez cette partie, par exemple.

if(!(ptr=malloc(sizeof(struct SymEntry)))) return false; 
if(!(ptr->Name=string)) 
{ 
    printf("if(!(ptr->Name=string))\n"); 
    free(ptr); 
    return false; 
} 
ptr->Name = string; 

Il est incohérent. Vous lancez le retour de malloc pour AnEntry ci-dessus, mais pas ce malloc. Faites l'un ou l'autre, mais ne le mélangez pas. Mieux encore, écrivez-le d'une manière qui n'a pas besoin d'un casting du tout.

Vous ne devez pas affecter de valeurs dans les instructions if. Bien qu'il soit toujours clair ce que vous voulez faire dans le cas malloc, l'intention est obscurcie dans l'attribution de chaîne. Surtout que c'est superflu. Quand if est évalué à true, ptr est immédiatement libéré. Quand il évalue à faux, la même attribution est exactement faite à nouveau. De plus, dans ce cas, il empêche une optimisation évidente.

est ici le même réécrite de code:

if (string == NULL) 
{ 
    printf("string == NULL\n"); 
    return false; 
} 
ptr = malloc(sizeof *ptr); 
if (ptr == NULL) 
{ 
    return false; 
} 
ptr->Name = string;