segment error?

I am sopossed to edit a prebuilt program to include some extra stuff for a hash table, it is not done yet. When i compile it, i get errors on line 139.


"operands of = have illegal types 'array 69 of char' and 'pointer to int'
the left hand side of the assignment can't be assigned to
148 operands of = have illegal types 'array 69 of char' and 'pointer to void'
148 the left hand side of the assignment can't be assigned to"

Code:

---



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

#define AtoM  1
#define N_Z    2
#define STRINGSHOT 69

//#define debug

  typedef struct HashCells{
      struct HashCells *prev;
      char value[STRINGSHOT];
      struct HashCells *pnext;
  } Cell;

  Cell *g_AtoDHashCell = 0;
  Cell *g_EtoHHashCell = 0;
  Cell *g_ItoLHashCell = 0;
  Cell *g_MtoPHashCell = 0;
  Cell *g_QtoTHashCell = 0;
  Cell *g_UtoZHashCell = 0;


  void        Init_HashTable  ( void                                                );
  void        AddCell                ( char lastname[],char firstname[],char street[],char city[]);
  int        HashCode                (  char lastname[] );

  void    AssignCell      (char lastname[],char firstname[],char street[],char city[] );
  void    PrintCells      ( void                      );
  void readlist(void);


    int main()
  {
 
      Init_HashTable();
      readlist();
 
 
 
  /*
 
  */
      PrintCells();
 
      return 0;
  }


    void readlist(){
      FILE *filelist;
      filelist=fopen("filelist.txt", "r");
      char line[STRINGSHOT];
      char street[STRINGSHOT], city[STRINGSHOT];
      char firstname[STRINGSHOT], lastname[STRINGSHOT];
 
 
      while(fgets(line, sizeof(line), filelist)!=NULL){
        sscanf(line,"%[^ ] %[^;];%[^;];%[^;\n]",firstname,lastname,street,city);
        /*printf("first name: %s\n", firstname);
        printf("last name: %s\n", lastname);
        printf("street: %s\n", street);
        printf("city: %s\n", city);*/
        AddCell(lastname,firstname,street,city);
      }
      fclose(filelist);
 
  }



    void Init_HashTable( void )
  {
 
 
      Cell *new;
 
      new                        = malloc( sizeof(Cell) );
      new->pnext                = NULL;
      new->prev                  = NULL;
      g_AtoDHashCell = new;
      g_EtoHHashCell=new;
      g_ItoLHashCell=new;
      g_MtoPHashCell=new;
      g_QtoTHashCell=new;
      g_UtoZHashCell=new;
  }

    void AddCell( char lastname[],char firstname[],char street[],char city[]){
      switch( HashCode( lastname)){
     
        case 1:
            AssignCell( lastname,firstname, street,city );
            break;
     
        case 2:
            AssignCell( lastname,firstname, street,city );
            break;
     
        case 3:
            AssignCell( lastname,firstname, street,city );
            break;
     
        case 4:
            AssignCell( lastname,firstname, street,city );
            break;
     
        case 5:
            AssignCell( lastname,firstname, street,city );
            break;
     
        case 6:
            AssignCell( lastname,firstname, street,city );
            break;
     
        default:
            printf(" Unable to hash.\n");
     
      }
  }

    void AssignCell( char lastname[],char firstname[],char street[],char city[] )
  {
 
      Cell *pLocal;
      Cell *new;
 
      pLocal = g_AtoDHashCell;
 
      while( pLocal->pnext != NULL )
        pLocal = pLocal->pnext;
 
        //
        // store value
        // No malloc test !!!!
      pLocal->value = (int *)malloc( sizeof(lastname) );
      strcpy( (char *)pLocal->value, lastname );
 
 
        //
    // Setup for next entry
      new = malloc( sizeof(Cell) );
      pLocal->pnext        = new;
      new->pnext                = NULL;
      new->value      = NULL;
      new->prev                  = pLocal;
 
 
 
  }

    int HashCode( char lastname[] )
  {
 
      int HashKey;
      char firstltrlast[1];
  /*copies the first character of the last name into the sring firstltrlast*/
      strncpy(firstltrlast, lastname,1);
 
      /*Divides Names that start with ABCD into hash 1*/
      if(strpbrk(firstltrlast,"ABCD")!=NULL){
        HashKey=1;
      }
      /*Divides Names that start with EFGH into hash 2*/
      if(strpbrk(firstltrlast,"EFGH")!=NULL){
        HashKey=2;
      }
      /*Divides Names that start with IJKL into hash 3*/
      if(strpbrk(firstltrlast,"IJKL")!=NULL){
        HashKey=3;
      }
      /*Divides Names that start with MNOP into hash 4*/
      if(strpbrk(firstltrlast,"MNOP")!=NULL){
        HashKey=4;
      }
      /*Divides Names that start with QRST into hash 5*/
      if(strpbrk(firstltrlast,"QRST")!=NULL){
        HashKey=5;
      }
      /*Divides Names that start with UVWXYZ into hash 6*/
      if(strpbrk(firstltrlast,"UVWXYZ")!=NULL){
        HashKey=6;
      }
        // Assume string[0] always is A thru M
      return (HashKey);
  }

    void PrintCells( )
  {
 
      Cell *pLocal;
      int iCount = 0;
 
      pLocal = g_AtoDHashCell;
 
        //
        // Follow the trail
      while( pLocal->pnext != NULL )
      {
        printf( " Value of Cell #%d = %s \n", ++iCount, (char*)pLocal->value);
        pLocal = pLocal->pnext;
      }
 
  }

---

Code:

---

char firstltrlast[1];
  /*copies the first character of the last name into the sring firstltrlast*/
      strncpy(firstltrlast, lastname,1);

---

Why are you trying to have an array of one character? Also, a "string" can never fit into a single character, unless that single character is the null character and nothing more. In other words, you either want just a single character, or you need to expand the size of your array.

You also need to start compiling as C instead of C++, or, if you already are, then you should stop typecasting your allocation functions. There's a FAQ on it. Speaking of malloc, stop trying to allocate space for arrays:

Code:

---

pLocal->value = (int *)malloc( sizeof(lastname) );

---

It's an array, not a pointer. They're not the same thing.


Quzah.

> pLocal->value = (int *)malloc( sizeof(lastname) );
pLocal->value is an array of char. There is no need to allocate memory for an array of char, so you can remove this line of code. If it was a pointer to char, then you would need to allocate memory.

> strcpy( (char *)pLocal->value, lastname );
The cast is unnecessary. All you need is:

Code:

---

      strcpy( pLocal->value, lastname );

---

> char firstltrlast[1];
I'm guessing that you need to make room for the string terminator, so this should be at least of size 2:

Code:

---

      char firstltrlast[2];

---

> strncpy(firstltrlast, lastname,1);
strncpy() doesn't automatically add the string terminator, so you would need to add the code to append the string terminator:

Code:

---

      strncpy(firstltrlast, lastname,1);
      firstltrlast[1] = '\0';

---

1. don' mix declarations and statements

Code:

---

      FILE *filelist;
      filelist=fopen("filelist.txt", "r");
      char line[STRINGSHOT];
      char street[STRINGSHOT], city[STRINGSHOT];
      char firstname[STRINGSHOT], lastname[STRINGSHOT];

---

2. Don't cast malloc
pLocal->value = (int*)malloc( sizeof(lastname) );
3. Don't cast malloc incorrectly
4. Don't try to assign value to array
pLocal->value is array you cannot modify its address
the return value of the malloc can be stored only in the pointer
array and pointer is not the same

i'll try that stuff out.


to tell you the truth, my teacher wrote all that malloc stuff and casting stuff.

and yes i was trying to have a string of a single character because the program was giving me lip about anything larger

Code:

---

char firstltrlast[1];
  /*copies the first character of the last name into the sring firstltrlast*/
      strncpy(firstltrlast, lastname,1);

---

ok thanks so much for your input everyone, you really saved me.

It works, but i think its shoving all names into each one of the 6 buckets. because no matter what i change the "pLocal = g_AtoDHashCell;" to (Q to T or whatever) it still prints all the 75 names and stuff.

Also, how can I delete a specific name from the table?

Code:

---

    void PrintCells(){
      Cell *pLocal;
      int iCount = 0;
 
      pLocal = g_AtoDHashCell;
 
      while(pLocal->pnext != NULL){
        printf("Value of Cell #%d:\n",++iCount);
        printf("    %s, %s\n",pLocal->lname,pLocal->fname);
        printf("        %s, %s\n\n",pLocal->streetname,pLocal->cityname);
        pLocal = pLocal->pnext;
      }
  }

---

Most of the time C code is indented such that global entities such as function definitions are not indented.

Code:

---

int x;

void f(void);

void f(void) {
    int x;
}

---

The way you have it is fine, though.

This code could use some improving.

Code:

---

    int HashCode(char lastname[]){
      int HashKey;
      char firstltrlast[STRINGBUF];
      strncpy(firstltrlast, lastname,1);
      firstltrlast[1] = '\0';
 
      if(strpbrk(firstltrlast,"ABCD")!=NULL){
        HashKey=1;
      }
      if(strpbrk(firstltrlast,"EFGH")!=NULL){
        HashKey=2;
      }
      if(strpbrk(firstltrlast,"IJKL")!=NULL){
        HashKey=3;
      }
      if(strpbrk(firstltrlast,"MNOP")!=NULL){
        HashKey=4;
      }
      if(strpbrk(firstltrlast,"QRST")!=NULL){
        HashKey=5;
      }
      if(strpbrk(firstltrlast,"UVWXYZ")!=NULL){
        HashKey=6;
      }
      return (HashKey);
  }

---

HashKey isn't initialized, so if none of the ifs is true the function returns an undefined value. I would initialize it to zero. Also, those ifs could be else-ifs.

That could also be the source of your problem. Because those ifs aren't else-ifs, it doesn't matter if there's an 'M' in the string; if there's a 'Z' as well, the function will return 6 (indicating 'Z').

The strncpy could easily be replaced with

Code:

---

firstltrlast[0] = lastname[0];

---

The only reason firstltrlast needs to exist at all is because strpbrk needs a string argument. If you use strchr() instead, you could replace this

Code:

---

      if(strpbrk(firstltrlast,"ABCD")!=NULL){

---

with this

Code:

---

      if(strchr("ABCD", lastname[0])!=NULL){

---

and get rid of firstltrlast completely.

Now, this switch statement is also pretty pointless.

Code:

---

    void AddCell(char lastname[],char firstname[],char street[],char city[]){
      switch(HashCode(lastname)){
        case 1:
            AssignCell(lastname,firstname,street,city,1);
            break;
     
        case 2:
            AssignCell(lastname,firstname,street,city,2);
            break;
     
        case 3:
            AssignCell(lastname,firstname,street,city,3);
            break;
     
        case 4:
            AssignCell(lastname,firstname,street,city,4);
            break;
     
        case 5:
            AssignCell(lastname,firstname,street,city,5);
            break;
     
        case 6:
            AssignCell(lastname,firstname,street,city,6);
            break;
     
        default:
            printf("Unable to hash.\n");
      }
      return;
  }

---

You can replace the whole thing with

Code:

---

int hashnum = HashCode(lastname);
if(hashnum >= 1 && hashnum <= 6) {
    AssignCell(lastname,firstname,street,city,hashnum);
}
else puts("Unable to hash.");

---

unless, for some reason, you need to use a switch statement, in which case you can use the fall-through behaviour

Code:

---

      switch((hasnum = HashCode(lastname))){
        case 1:
        case 2:
        case 3:
        case 4:
        case 5:
        case 6:
            AssignCell(lastname,firstname,street,city,hashnum);
            break;
     
        default:
            printf("Unable to hash.\n");
      }

---

unless you can't use a variable for some reason, in which case that's a stupid assignment. :)

Also, you might want to check the return value of sscanf() to see if it encountered any errors.