Cant pass a correct Dynamic array of structs in c

Question

Im working on a project for school and I can't pass a dynamic array of structs to another function in c. The function is just supposed to check an element of the struct and return alreadyadded if that element of the array is equal to the current one. Im also having trouble declaring the function that accepts the calloc array. Any help would be appreciated! Ive been looking for a solution for this all night.

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <stdbool.h>
struct EDGETAG;
typedef struct
     {
      char c;
      bool isVisited;
      struct EDGETAG* p;
     } VERTEX;

typedef struct EDGE
    {
    VERTEX* v;
    struct EDGETAG* q;
    } EDGE;

int main(int argc, char* argv[])
   {
    int a;
    struct VERTEX *vert = (VERTEX*)calloc(100, sizeof (VERTEX*));
    char s;
    int count = 0;
    FILE* input = fopen(argv[1],"r");
    while((a = fgetc(input)) != EOF)
       {
        if(isspace(a)==0)
         {
         s = a;
         printf("%c ",s);
         determiner(s,vert,count);
         count++;            
         }
      }
return 0;
}

and the called function

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <stdbool.h>


typedef struct VERTEX
    {
    char c;
    bool isVisited;
    struct EDGETAG* p;
    } VERTEX;


typedef struct EDGETAG
    {
   VERTEX* v;
    struct EDGETAG* q;
    } EDGE;


void determiner (char a, struct VERTEX *vert, int count)
    {
    int i;
    for(i=0;i < count; i++)
       {
       if(vert[i].c == a)
         {
         printf("%c allready added ",vert[i].c);
         return ;
         }
     else
       {
       VERTEX* new1 = (VERTEX*)malloc(sizeof(VERTEX));
       new1->c = a;
       vert[i] = *new1;
      }
    }
return ;

input is: A B B C E X C D A C output:A B B B allreadyadded C E X C D A C

Answer 1

You never actually allocate any vertexes.

struct VERTEX *vert = (VERTEX*)calloc(100, sizeof (VERTEX*));

Notice the sizeof(VERTEX*)? You've allocated enough space for 100 pointers to vertexes!

Answer 2

As well as the issue David Schwartz pointed out, an indented version of determiner shows another problem more clearly:

void determiner(char a, struct VERTEX *vert, int count)
{
    int i;
    for (i = 0; i < count; i++)
    {
        if (vert[i].c == a)
        {
            printf("%c allready added ", vert[i].c);
            return;
        }
        else{
            VERTEX* new1 = (VERTEX*)malloc(sizeof(VERTEX));
            new1->c = a;
            vert[i] = *new1;
        }
    }
    return;
}

Your current code looks at the first element of the array to see if it matches with a

• If it does match, it returns (good)
• If it doesn't match it overwrites the first element with new data (bad)

It then loops and does the same thing with the second element etc.

To fix this, move the code that adds a new element outside of the checking loop. If the checking loop finishes, then it hasn't been found, so can then be added.

Now, you have a second problem - you always increase count no matter whether determiner found a match or not. You should change determiner to return a value indicating whether the item was added, then use that in main to increase count or not.

Edit: Also note - you don't need (or want) to allocate the new node using malloc, if you are just going to copy its contents into vert[i]. Currently you are leaking memory - much easier to just update vert[i].c directly.

Answer 3

You are calling in the first file

typedef struct
{
  char c;
  bool isVisited;
  struct EDGETAG* p;
} VERTEX;

Later down, you have

struct VERTEX *vert = ...

struct VERTEX is not a type in the first file. VERTEX is defined as a type. You should change the struct type to

typedef struct VERTEX // note the VERTEX here
{
  char c;
  bool isVisited;
  struct EDGETAG* p;
} VERTEX;    

This is already done in the second file.

As a general style guide, you should include the struct declarations for VERTEX and EDGE in a separate header file and include it in both your .c files.

Answer 4

There are many issues with your code, but I will add my 2 cents to the answers above, since I believe it might be helpful:

1. As @David already noticed, you are instantiating an array of pointers:

   // this should be a struct VERTEX**, if you want to
   // have a "pointer to the first pointer to VERTEX"
   struct VERTEX *vert = (VERTEX*)calloc(100, sizeof (VERTEX*));
   

   but you (most likely) wanted to instantiate an array of VERTEXes:

   // you probably want space for 100 x sizeof(VERTEX),
   // so that vert is simply a "pointer to the first VERTEX".
   // ALSO: don't cast the result of calloc/malloc 
   struct VERTEX *vert = calloc(100, sizeof(VERTEX));
   

   vert is not a pointer to the first VERTEX element of the allocated array. You don't need to allocate individual VERTEX elements and vert is never going to be null as you iterate, so you need to keep track of the count yourself.

2. count shouldn't be incremented inside the main body, since you don't know if you actually added the element or not.

3. Your determiner function is not correct; you seem to be adding a new instance in each loop iteration. I would also return the count value from the function when it actually adds the element:

   void determiner (char a, struct VERTEX *vert, int *count)
   {
       for(int i = 0; i < count; i++)
       {
           if (vert[i].c == a)
           {
               printf("%c already added ",vert[i].c);
   
               // no need to increase count
               return;
           }
       }
   
       // if we are here, we didn't find the element,
       // so this is where you actually initialize it
       // and increase count
   
       vert[count]->c = a;
       count++;
   }
   

   When calling the function, pass the pointer to count and let the function increase it:

   determiner(s, vert, &count);
   

4. When working with graphs, it's common to use a linked list, i.e. make nodes point to other nodes. In that case you don't allocate an array, but use malloc for each node separately and jump forward until you reach null. Try googling for an example of how to implement a graph in C to get a better idea of the proper approach.