Array of strings question

Hey all, I'm working on a homework assignment that reads binary files and performs queries on them. I came across a problem with trying to not print out duplicate results that were read in from a binary file. I'm using fread() to get the name of a person then i want to store that name in an array of strings so when i go to print i can check to make sure it doesnt match any of the other names. The problem im having is that the array is filling up with pointers that all point to the last object read in. Heres my code.

Code:

---

#include<stdio.h>
#include<string.h>
#include<stdlib.h>
int projcmd(char sPtr[])
{
  FILE *finp;
  FILE *finpdat;

  char *relname;
  char *filename;
  char *attrname;
  char s[] = "S";
  char is[] = "I";
  char project_name;
  char eof_test;
  int i,j,val, s_length, pos, atr_length, seek_val, tot_length, advance_val;

  char *sori, *cslen, *sori_F;
  char *attname;
  char sch_string[80];
  char os[80];
  char test_string[80];
  char **dupArray;
  char temp[80];
  char *reference;
  char *tPtr;

  tPtr = strtok(sPtr, " ");
  tPtr = strtok(NULL, " ");
  relname = tPtr;
  tot_length = 0;
  atr_length = 0;
  seek_val = 0;
  advance_val = 0;

  attrname = strtok(NULL, " ");
  j = 0;
  while(attrname[j] != '\n')
    j++;

  attrname[j] = '\0';

  printf("%s %s\n", relname, attrname);

  strcat(relname,".sch");

  printf("%s\n", relname);
  if((finp = fopen(relname, "r")) == NULL)
    {
      printf("error");
      exit(1);
    }

  fscanf(finp, "%d", &val);
  pos = ftell(finp);



  for(i=0;i<val;i++)
    {
      fscanf(finp, "%s", attname);
      fscanf(finp, "\t%c", &sori);
      fscanf(finp, "\t%d", &s_length);
      tot_length = tot_length + s_length;

      if((strcmp(attname, attrname))==0)
        {
          atr_length = s_length;
          seek_val = tot_length - s_length;
          sori_F = sori;

        }

    }
 advance_val = tot_length - atr_length;
 j=0;
 while(relname[j]!= '.')
  j++;
 relname[j] = '\0';



    fread(test_string,1,atr_length, finpdat);
    strcpy(temp,test_string);
File Edit Options Buffers Tools C Help                                               
 strcat(relname, ".dat");
 finpdat = fopen(relname, "r");


 fseek(finpdat, (seek_val), SEEK_CUR);
 j = 0;

 dupArray = (char**)calloc(sizeof(char),5);                    //  The strings print correctly
 while((fread(&eof_test, sizeof(char), 1, finpdat))!= NULL)//  so i know the error is so
  {                                                                              //  somewhere down here
    fseek(finpdat, -1, SEEK_CUR);
    fread(test_string,1,atr_length, finpdat);
    strcpy(temp,test_string);
    dupArray[j] = temp;
    j++;
    printf("%s\n",temp);
    fseek(finpdat,advance_val,SEEK_CUR);

  }
 for(i=0;i<j;i++)
  {
    printf("%s\n", dupArray[i]);

  }



}

---

I should mention down here that the correct results print to stdout just not to the array

Perhaps because each element of dupArray points to the same temp storage as in

Code:

---

dupArray[j] = temp;

---

Consider what will happen in a few months or so when your projects amount to several hundred or several thousand lines of code. Will you show up here and post the whole thing, explaining "train A and train B, train A coming from a small port in the south carrying agriculture, of which the fruit I place on top so it isn't squished. Then there is a communication system between the trains, which is used for various purposes. Neither of them are on the same track, so I don't know why they collide."

I could, and probably someone eventually will, come along and extract the relevant as opposed to irrelevant information. However, if it was really important to me to solve the problem (and by me, I think I now mean you), I might decide to do that myself, first, before I post anything.

But how! you ask. Well, that's a challenge itself -- but it might contain the solution! I will honestly admit that MOST of the questions I set out to ask on cboard get solved before they are posted -- that is, I actually sit there composing a "new thread", in the little "compose new thread" box, and by the time I get the question formulated as specifically as I can, MOST of the time I realize the answer. Honest. I can also honestly say that I have NEVER had to post a piece of code this long and I can tell you FOR A FACT that you do not need to either.

To work on a problem like this, you need to familiarize yourself with all the steps involved (we call this "problem solving") by breaking them down and dealing with them one at a time. How many times, while working on this, have you written A SHORT, SEPARATE PROGRAM to check your understanding of the commands and concepts involved? If the answer is none, your attitude or methodology MUST change in time. So how about now? This is the natural form of computer programming anyway.

Most likely no one cares about or is interested in your project. However, there is a chance someone will take an interest in a problem, if you present it properly.

Best of all, almost certainly someone can answer a specific question ("is this the correct use of ???), but you don't have one yet. If you can write a short (25 lines or less) piece of code that is A COMPLETE, SELF-CONTAINED PROGRAM and explain what you wanted each significant block to accomplish, I promise I will solve your problem. If you don't do it first, that is.

I know it seems a hassle at first, like you are wasting time. But eventually you will recognize where time really gets wasted, and where it does not. Plus your coding (as opposed to pondering and hand-wringing) skills will naturally improve with every

Code:

---

int main () {

---

you start.

Or you could just listen to itCbitC, s/he's probably right ;)

Well, I was going to just post that area around the while loop but I just wanted to show my algorithm for moving around in the binary file. I have done several mini test programs and they do help I agree, but I guess the problem I'm having here is not the syntax but maybe help with an algorithm to read names from a binary file and print them to stdout without repetitions. I'm completely stumped if i cant do it by an array of strings.

Also quick question, in another source page i have for this project I keep getting bus error or segfaults from trying to declare new variables. Particularly FILE pointers. Anyone ever experience this.

Quote:

---

Originally Posted by FortinsM3

Well, I was going to just post that area around the while loop but I just wanted to show my algorithm for moving around in the binary file. I have done several mini test programs and they do help I agree, but I guess the problem I'm having here is not the syntax but maybe help with an algorithm to read names from a binary file and print them to stdout without repetitions. I'm completely stumped if i cant do it by an array of strings.

---

What all this implies to me is that the binary has a particular structure which you are trying to deal with and I just can't reconstruct it in my head by reading your code. Or at least I'm not even gonna try. That doesn't mean I want a copy or anything that much, but there is nothing wrong with abstracting a bit. I also cannot test the code to see how it runs. It now sounds like you are having more problems than the one itCbitC already solved. I doubt very much there is anything like "an algorithm to read names from a binary file and print them to stdout without repetitions", particularly if the repetitions are due to a mistake in your code and not the binary file.

Quote:

---

Originally Posted by FortinsM3

Also quick question, in another source page i have for this project I keep getting bus error or segfaults from trying to declare new variables. Particularly FILE pointers. Anyone ever experience this.

---

This will happen if the new variables are declared after you compile, because FILE pointers should be used when the program runs.

ps. are you aware you can use "rb" with fread?

No, rb is read byte I'm assuming. You are correct, there is a structure and it changes depending on what attribute we need to produce the results for. But I've tested this part seperately and it prints the outputs perfectly to stdout. There is repetitions in the bin file and its our programs job to just print out 1 if there happens to be multiple. We do not know how many sets of tuples there will be in a given binary file. So thats why I cant think of a way to save all these somewhere then when I run a print function I can weed out the duplicates. Thanks for taking the time MK, if you need me to provide a mini program like you said I can try but its difficult with all the different scenarios I have to account for.

Quote:

---

Originally Posted by FortinsM3

No, rb is read byte I'm assuming. You are correct, there is a structure and it changes depending on what attribute we need to produce the results for. But I've tested this part seperately and it prints the outputs perfectly to stdout. There is repetitions in the bin file and its our programs job to just print out 1 if there happens to be multiple. We do not know how many sets of tuples there will be in a given binary file. So thats why I cant think of a way to save all these somewhere then when I run a print function I can weed out the duplicates. Thanks for taking the time MK, if you need me to provide a mini program like you said I can try but its difficult with all the different scenarios I have to account for.

---

"rb" is read binary -- I don't recall the difference and it probably doesn't matter here. This sounds like more of a conceptual problem. It may be that the only way to deal with this is the obvious. But I would not keep the duplicates, since eliminating them will be the same process anyway; you have to compare each value against all the previous values. (Or do you mean only duplicates that happen one after another?)

The order does not matter, if there is more then one of the same attribute it needs to not be printed. But lets say I read all these strings in successfully from the bin file through loop until nothing else can be read. I copy each name(attribute) from the string test_string to temp, so after each iteration the dupArray should have the newest value that was stored in temp, now stored in dupArray right? wrong ha. It's ovbiously a pointer issue because dupArray just has 5 pointers to the last value stored in temp.
I'm so stumped.

Code:

---

while((fread(&eof_test, sizeof(char), 1, finpdat))!= NULL)
  {                                                                           
    fseek(finpdat, -1, SEEK_CUR);
    fread(test_string,1,atr_length, finpdat);
    strcpy(temp,test_string);
    dupArray[j] = temp;
    j++;
    printf("%s\n",temp);
    fseek(finpdat,advance_val,SEEK_CUR);

  }

---

Did you try to rectify the code I pointed out or is this another issue you're having?

Oh my gawd look!

Quote:

---

Originally Posted by itCbitC

Perhaps because each element of dupArray points to the same temp storage as in

Code:

---

dupArray[j] = temp;

---

---

The address of temp does not change! So every element of dupArray will point to the same place, which will be whatever you put there last. You have to allocate memory for all those strings, not just one.

ps. were you just ignoring itCbitC?!!? Or you were just afraid to ask?
pps. [edit] okay so you did allocate it, but guess what happened to that memory when you moved the pointer? It LEAKED! You can't free it or access it but it still takes up space...

Thanks for clarifying the issue MK27; guess the o/p is stuck at the same place.
Store each string read from the file in a separate location and point dupArray at it.

Code:

---

dupArray[j] = strdup(test_string);

---

there's no need for temp[]

Quote:

---

The address of temp does not change! So every element of dupArray will point to the same place, which will be whatever you put there last. You have to allocate memory for all those strings, not just one.

---

Thats all I needed. Would it be something like this?

Code:

---

temp = malloc(100);
inside the loop

---

Quote:

---

Originally Posted by FortinsM3

Thats all I needed. Would it be something like this?

Code:

---

temp = malloc(100);
inside the loop

---

---

Yep. In that case you get rid of your original calloc string, to avoid the LEAKING issue I mentioned above.

Ooops, sorry itcbitc i had my reply window up before you posted that. Well that code just gives me segfaults now, i think im calling it a night but thanks for the help guys.

post the updated code!

Hey guys I woke up early this morning and finished this code. Check out the working changes, and thanks to ItCbitC and MK27 for getting me over this hump. I owe you two my life!
p.s. figuring out that non duplicate algo was a pain in the rear

Code:

---

dupArray = (char**)malloc(val*sizeof(char*));

  fseek(finp_dat, seek_val, SEEK_SET);
  while((fread(&eof_test, sizeof(char), 1, finp_dat))!= NULL)
    {
      fseek(finp_dat, -1, SEEK_CUR);

      fread(test_string, sizeof(char),len, finp_dat);
      dupArray[j] = strdup(test_string);

      j++;

      fseek(finp_dat, advance_val, SEEK_CUR);
    }

  for(i=0;i<5;i++)
    {
      for(k=i;k<5;k++)
        {
          if((i!= k) && ((strcmp(dupArray[i], dupArray[k])==0)))
            {
              dupArray[k] = "dup";
              //printf("%s\n", dupArray[k]);
            }
        }
    }
  for(i=0;i<5;i++)
    {
      if(dupArray[i]!= "dup")
        printf("%s\n", dupArray[i]);
    }



    for(i=0;i<val;i++)
      {
        free(dupArray[i]);
      }
    free(dupArray);
}

---

Quote:

---

dupArray[k] = "dup";

---

when you will free this pointer - your program will crash

I agree with vart, dupArray[k] now points to a string literal so trying to free() a pointer that has not been allocated dynamically is undefined behaviour. IMHO a better design would be to free(dupArray[k]) and then set it to NULL. Doing it this way, conditionally print only those elements that are not NULL.