Thread: segment error?

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);

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.