0

The following program stores every word and then prints them with a number of occurrences.
Global typedef declaration:

typedef struct {
    char * word;
    int occ;
}
words;
words *data=NULL;

I have a problem with the search function. I've created a function returning int that looks like this: (max is the constantly updated size of an array of structures, that's why I call search function after EOF is reached.)

int search(char *word,int max)
{
    int i;
    for(i=0; i<max; i++)
    {
        if(!strcmp(data[i].word,word)) return i;
    }
    return -1;
}

But I noticed I'm supposed to write a search function having that prototype:

struct abc *find(char *word)

So I've created the following code:

struct words *findword(char *word)
{
    struct words *ptr;

    for (ptr = data; ptr != NULL; ptr++) {      /* IS THE STOP CONDITION OK? */
        if (strcmp(word, ptr->word) == 0)
            return ptr;
    }
    return NULL;          

}

And I receive many errors during compilation:

reverse.c: In function ‘findword’:

reverse.c:73: warning: assignment from incompatible pointer type

reverse.c:73: error: increment of pointer to unknown structure

reverse.c:73: error: arithmetic on pointer to an incomplete type

reverse.c:74: error: dereferencing pointer to incomplete type

reverse.c: In function ‘main’:

reverse.c:171: error: ‘which’ undeclared (first use in this function)

reverse.c:171: error: (Each undeclared identifier is reported only once

reverse.c:171: error: for each function it appears in.)

make: * [reverse.o] Error 1


which is an int variable assigned to the return of my firstly written search function. The error with which is easily fixed, but I don't know how to replace that (solution working with my base search function):

data[which].occ++;

How to fix it so that it'll work with my new approach to search?


EDIT

main() added:

int main(int argc, char **argv)
{
    char *word;
    words *temp;
    int c,i,num;
    /*int which;*/
    FILE *infile;

    if(argc!=2) {}      
    if((infile=fopen(argv[1],"r"))==NULL) {}
    num=0;
    while(1)
    {
        c=fgetc(infile);
        if(c==EOF) break;
        if(!isalpha(c)) continue;
        else ungetc(c,infile);
        word=getword(infile);
        word=convert(word);
        /*which=search(word,num);*/ 
        if(findword(word))
        {
            if(!(temp=realloc(data,sizeof(words)*(num+1))))
            {}
            else
                data=temp;
            data[num].word=strdup(word);
            data[num].occ=1;
            num++;
        }
        else
            data[which].occ++;

        free(word);
    }
    sort(num-1);
    for(i=0;i<num;i++)
    {}
    free(data);
    if(fclose(infile))
    {}  
    return 0;
}

I've left {} for the irrelevant pieces of code eg. error handling.


EDIT2 The things I'm asking for above, are fixed. However, I get a seg fault now. I'll give a link to the whole code, I don't want to put it in an edited post since it'd create a big mess. Seg fault is caused by lines 73 and 152 (strcmp is not working somehow). Hope that full code will be easier to understand. FULL CODE

7
  • Why not just move that int max parameter to a global variable and use the first version of the code? The second version is going to crash because the pointer is going to be incremented and incremented beyond the end of the data[] array. Commented Sep 18, 2012 at 12:33
  • @AlexeyFrunze I can't use the first function since it returns int and I should return particular place in an array of structures. Ad.2 - How to fix that, so that the pointer stops incrementing at the end of constantly expanding array of structures? Commented Sep 18, 2012 at 12:52
  • Return &data[i] instead of i, what's the problem with that? You can't fix that pointer other than by 1) introducing an element counter (or a pointer to the last element) and comparing the currently examined location with that OR, which I don't recommend, 2) embedding a special indicator in the last element of data[] and checking it. Commented Sep 18, 2012 at 13:01
  • Could you add the code from your main() function please? Commented Sep 18, 2012 at 13:26
  • So only main knows how big your array of words is. You need to either store that gobally alongside data, or store a magic value to mark the end: then, you can write a loop that stops without overrunning the array, and without passing the size into your findword function. Commented Sep 18, 2012 at 14:00

3 Answers 3

2

The problems are with your findword function, lets go through all the lines

struct words *ptr; 

This is not what you ment to do. The typedef you used in defining the structure allows you to not need to write struct anymore. This is why you're getting the error: reverse.c:73: error: increment of pointer to unknown structure. What you want is just:

words *ptr;    

Next, the loop:

for(ptr=data; //This is fine, you're assigning your local ptr to the global data. I assume that's something valid

ptr != NULL; //That could OK too... we can loop while ptr is not NULL
ptr++)       //This line makes no sense... 

You may want to look up how for loops work again, the point is you're incrementing something until it hits a condition. ptr++ will move where you're pointing too, so you'll no longer be pointing to your structure.

I need to see your main() function to understand what you're trying to accomplish, but based on the prototype you have to follow, I think the easiest solution would be something like:

void main()
{
    // init your vars
    bool more_words_to_add = true;
    words *ptr = NULL;
    int i;

    // populate your array of words
    while(more_words_to_add) {
        for(i = 0; i<max; i++) {
          if(ptr = findword("word"))  //if we find the word
            ptr->occ++;  //increment the number of times we found it
          else {
            //I don't know what you want to do here, it's not clear what your goal is.
            //Add the new word to your array of words and set occ to 1,
            //then increment max because there's one more word in your array?
          }
        }
        //get the next word to fine, or else set more_words_to_add = false to break
    }
}

If this type of solution is what you're looking to do, then you can adjust your findwords function to be very simple:

struct words *findword(char *word)
{
    words *ptr = data;
    if (strcmp(word, ptr->word) == 0)
        return ptr;
    return NULL;
}  

EDIT: For your new error I suspect the problem is with your memory allocation, see this short example of using your structure:

words *findword(char *word)
{
    words *ptr = data;
    if(strcmp(word, ptr->word) == 0)
      return ptr;
    return NULL;
}

int main(){
    words *ptr;

    data = realloc(data, sizeof(words));
    data->word = "hello";                //DO NOT SKIP THESE LINES
    data->occ = 0;                       //DO NOT SKIP THESE LINES

    if(ptr = findword("hello")) {
      ptr->occ++;
      printf("I found %d %s's\n",ptr->occ, ptr->word);
    }
} 

mike@linux-4puc:~> ./a.out 
I found 1 hello's

You can see here that you need to alloc some memory for the global structure then you can store data in it and pass pointers to it.

EDIT 2:

Your main() code does this:

if((ptr = findword(word)))
{
     //do stuff
}
else
  ptr->occ++;

That's not going to work because if findword() fails it returns NULL, so in the if check ptr is set to NULL, then in the else you're trying to deference NULL. If (and keep in mind I'm not really reading your logic so this is up to you) you really want to increment ptr->occ if a word is not found then you want this instead:

if(findword(word))
{
     ptr = findword(word);
     //do stuff
}
else
  ptr->occ++; //increments the current ptr's occ, no new ptr was assigned.
Sign up to request clarification or add additional context in comments.

6 Comments

Thank You. I've removed all the errors (following Your post) from compiling stage. However, now I'm getting Segmentation fault and valgrind indicates two lines causing error: if (strcmp(word, ptr->word) == 0) and if(findword(word)) from main Any advice what is wrong with these?
Your code is getting a little hard to think about. Can you clean it up to the current version since you're done to these new problems? I suspect the problem is with your memory allocation, see my EDIT on my post as to how to use that global structure and see if that helps.
I'll give a link to the whole code, I don't want to put it in an edited post since it'd create a big mess. Seg fault is caused by lines 73 and 152 (strcmp is not working). Hope that full code will be easier to understand. link
The problem is you didn't follow my example. :) I re-edited it, see the //DO NOT SKIP THESE LINES part? What you're doing in your code is allocating memory but not initializing it to anything. That makes data->word a bad pointer. Now in my code I made it "hello" but you can just make it an empty string "" or just do a memset of 0 or whaever you want, but you NEED to initialize before you use.
Thank You. Now it almost works - got oe new error on the marked line [link] (pastebin.com/SWrrMgFc) Should I repeat realloc for ptr as well?
|
2
for (ptr = data; ptr != NULL; ptr++) {    
/* IS THE STOP CONDITION OK? */

No. Your pointer just keeps getting incremented. The only thing that would make it NULL in that code is integer overflow. You could look at what it points to, and see if that is NULL, IF you preset the data area to 0's:

#define NUM_WORDS 100
data = calloc(NUM_WORDS,sizeof(words));

Or

#define NUM_WORDS 100
int bytes = NUM_WORDS * sizeof(words);
data = malloc(bytes);
memset(data,0,bytes);

....

for (ptr = data; ptr->word != NULL; ptr++) { 

If you don't want to preset the data area to 0 then you will have to pass the current amount of structs currently held in the data area to your function in order to know how much to loop.

Comments

1

There's no such thing as struct words in your program; there's an unnamed struct type, and a typedef words to that type. Either use struct words or words consistently.

You'll then need to replace

data[which].occ++;

with

result->occ++;

where result is the return value from your new search function.

2 Comments

I'm using words as You can see. I suppose it is wrong name then, what should I change to omit those compiler errors?
@PeterKowalski no, you're using struct words.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.