struct question

answered

The error message "left side of -> is not a pointer to struct or union" (details may vary) should tell you that the left side of ->, namely rule, is not a pointer to a struct. It is itself a struct; so you need to use rule.target to access members.

if I declare rule_t* rule then this would work right?

edited

You declare rules to be a pointer to a rule_t. That's fine and good, but at the moment rules points to somewhere just outside of Hoboken, and then when you try to access rules->dependency on the next line, the Mafia comes and gets you.

If you want to actually have a rule, you need to either malloc one for rules to point at, or declare one.

I see.. so the reason why is because I haven't declare rule_t?? how do I declare one?

oh and one more thing, when I put HashTable = HashTableConstuct(50);

it gives me this error:

/tmp/ccGGLIcr.o: In function `main':
memake.c:(.text+0x52): undefined reference to `HashTableConstruct'
collect2: ld returned 1 exit status

I already include the "hash.h" which is the interface of the hash table.. why wouldn't it work?

Quote:

---

Originally Posted by -EquinoX-

I see.. so the reason why is because I haven't declare rule_t?? how do I declare one?

---

by

Code:

---

rule_t rule; /* this is a rule_t */

---

as opposed to

Code:

---

rule_t *rule; /* this will point to a rule_t, if I ever have one to point to */

---

Quote:

---

Originally Posted by -EquinoX-

oh and one more thing, when I put HashTable = HashTableConstuct(50);

it gives me this error:

/tmp/ccGGLIcr.o: In function `main':
memake.c:(.text+0x52): undefined reference to `HashTableConstruct'
collect2: ld returned 1 exit status

I already include the "hash.h" which is the interface of the hash table.. why wouldn't it work?

---

Note the "ld returned" which means we're dealing with the linker. It compiled just fine -- but you need to include the library itself. Do you have the source for HashTableConstruct? If so, add it to the project. Do you only have the binaries (in .o, .obj, .lib, .dll, .a, or .so form)? If so, you'll need to link it in using the -l switch (on gcc, I forget what it is on MSVC).

what do you mean the source for HashTableConstruct? you mean the .c file??

Quote:

---

Originally Posted by -EquinoX-

what do you mean the source for HashTableConstruct? you mean the .c file??

---

That would be the source code, yes.

hmm..doesn't it seem weird to include the .c file?? should including the .h file works?

You don't include the .c file using #include; but you do need to turn that source into executable code!

I don't understand what you mean.. sorry

You have a .c file with code in for HashWhateverItWas(), right? You need to compile that file too. If you have a reasonable IDE, it will handle it for you if you create a "project" (or maybe "solution") and put all the files in it.

Your linker (ld) cannot find the function that you wrote called HashTableConstruct. You have yet to either write it, or link the file that has it inside with the rest of the project.

You can't link source code which calls functions that aren't written (or syntactically incorrect).

And files must be linked with the other source files that they are dependent on. For example if I had main.c and HashTableConstruct.c, and I linked only main.c, it might fail. This is because I neglected to link main.c and HashTableConstruct.c together. Using an IDE, I can fix this by simply adding HashTableConstruct.c to my project.

the hashtablecontruct is in hash.c and the interface is hash.h... on my hash.c file I already include the hash.h file.. and I don't see any way why this would fail?

Think for three seconds: when you compile main.c, does hash.c compile too? No? Then you need to do this:

Code:

---

gcc main.c hash.c -o main.exe

---

If you're using an IDE, then you have no excuse whatever: add hash.c to your project.

Your hash.c file needs to be in your project. Add the source file to your project folder and then relink (which will use your last successful compile). It should work.

okay I got it fixed now.. now it gives me this error:

hash.c:106: warning: assignment discards qualifiers from pointer target type
hash.c:113: warning: assignment discards qualifiers from pointer target type

what does it mean?

Probably you're turning a const * into a just plain *.

Quote:

---

hash.c:106: warning: assignment discards qualifiers from pointer target type
hash.c:113: warning: assignment discards qualifiers from pointer target type

---

It would be nice to see the relevant sections of this code.

oh yes.. I turn a const into plain.. what's the difference though? a constant can't be changed?

edited

Quote:

---

Originally Posted by -EquinoX-

oh yes.. I turn a const into plain.. what's the difference though? a constant can't be changed?

---

That's what constant means, yes. It's just a warning in C, but what it means is that you're going to get segfaults when it runs. So be careful with that.

Quote:

---

Originally Posted by -EquinoX-

okay back to the struct question then. say I have this struct:

Code:

---

struct _rule_t_{
  char** dependency;
  char** buildcmd;
  char* target;
};

typedef struct _rule_t_ rule_t;

---

and I have a hashtable which values are rule_t

how do I insert a rule_t into the hashtable?? I know I have a insert method... here's my insert function

Code:

---

boolean HashTableInsert(HashTable *this, char* key,  rule_t* value)
{
  list_t *temp;

  int_u4 hashval = StringHash(key) % this->size;

  /* Does item already exist? */
  for (temp = this->table[hashval]; temp != NULL; temp = temp->next){
    if (strcmp(key,temp->rule->target) == 0){
      RuleDestruct(temp->rule);
      temp->rule = value;
      return false;
    }
  }
  list_t *new_list;
  if ((new_list = malloc(sizeof(list_t))) == NULL)
    outOfMemory_(__LINE__, __FILE__, "struct node creation");
  new_list->rule = value;
  new_list->next = this->table[hashval];
  this->table[hashval] = new_list;
  this->length = this->length + 1;
  return true;
}

---

---

You have to decide what the "key" is going to be for your hashtable. Remember that keys have to be unique, so whatever unique thing is associated with your rule_t, that's what you'll use.

Need pointer lessons? Here you go:
http://cpwiki.sourceforge.net/A_pointer_on_pointers
It might enlighten you a little. Pointers are just variables that contains an address, nothing more, nothing less. It does not allocate memory.

And a constant variable is a variable whose value cannot change. That's what the const identifier is for - to tell you, and the compiler, that the value of the variable won't change. But if you do away with the const, then you're defeating the whole purpose of it, effectively making it useless to make it const in the first place. So the golden rule is that unless you have a very, very, very good reason to cast away the const, don't do it (hint: you probably don't).

edited

Code:

---

int chars_read = getline(&line, &line_length, fr)

---

The first argument of getline has to exist for this to work. There is no character array available at &line.

well I already declare line:

char* line = NULL;

the thing is that when I remove the HashTableInsert, it kind of works

or am I putting free at the wrong place?

Quote:

---

Originally Posted by Elysia

Need pointer lessons? Here you go:
http://cpwiki.sourceforge.net/A_pointer_on_pointers
It might enlighten you a little. Pointers are just variables that contains a virtual address, nothing more, nothing less. It does not allocate memory.

---

Try to avoid misinforming when attempting to inform; it will serve a greater purpose.

Quote:

---

Originally Posted by -EquinoX-

well I already declare line:

char* line = NULL;

the thing is that when I remove the HashTableInsert, it kind of works

---

And do you think the NULL pointer points at a character array?
And for that matter, you're using &line, which means that the string is being put into the memory address of line (not where line points!), but you've only got room for maybe three letters there, not to mention you won't ever get them back out, since how do you plan to reinterpret line as a char instead of a char*?

You can't store any information in an ordinary char* pointer that isn't pointing at allocated memory. You should either use an array, or dynamically allocate some memory for the pointer to point to.

I haven't read the rest of this thread, but since you have free(), I'm guessing you want to use the dynamic memory approach. Try something like this

Code:

---

char *line = malloc(256);

---

[edit] It seems a few posts appeared while I was typing this one up. [/edit]

You're doing wrong again. Twice, in fact.

Code:

---

      char* line = NULL;
      size_t line_length;
      int chars_read = getline(&line, &line_length, fr);

---

Did you read the link I gave you? It should give insight as to why this fails.
If you DID read it and didn't understand something, then ask.

doesn't actually get line fills the line for us?

Quote:

---

Originally Posted by -EquinoX-

doesn't actually get line fills the line for us?

---

Yes, but you have to have somewhere to put it. It will not allocate memory for you.

Yes, getline() should fill in the pointer for you.

--
Mats

Quote:

---

Originally Posted by tabstop

Yes, but you have to have somewhere to put it. It will not allocate memory for you.

---

If we assume the example here is a working one, then it will:
http://linux.die.net/man/3/getline

--
Mats

yeah..matsp.. thanks for pointing that out.. that's the getline what I mean

Quote:

---

Originally Posted by matsp

If we assume the example here is a working one, then it will:
http://linux.die.net/man/3/getline

--
Mats

---

Hey what do you know. How many versions of getline does that make, three I think? The C++ one doesn't work that way, does it? (All my docs say it needs room at the pointer, but who knows.)

sorry guys, I should have mentioned it to you before... so why do I keep get a seg fault even though that's right?

So we'll keep reading:

Code:

---

if (line[0] != '\t'){
          if (rule.target != NULL){
            rule.dependency = dep;
            rule.buildcmd = bld;
            rule.depSize = depIndex + 1;
            rule.bldSize = bldIndex + 1;
            HashTableInsert(t, rule.target, &rule);
            RuleDestruct(&rule);
          }
          char* pch = strtok(line, ":");
                int h;
        for (h = 0; pch != NULL; h++){
            if (h == 0){
            rule.target = pch;
            printf("Target %s\n", pch);
            bldIndex = 0;
            depIndex = 0;
            }

---

1. When we get here, I don't see how rule.target is set.
2. In your for-loop, I don't see how pch is going to change. (You don't change it inside the loop.) Edit: I lost a brace -- so found where pch changes; never mind.

okay rule.target is set? what do you mean? how can I set that?

and it seems that I am missing a free(line) here, but don't know where to put..

my point here is actually just to create a temporary rule which I can put into the HashTable.. do I have to do a malloc first? if yes then how much space do I have to malloc?

Quote:

---

Originally Posted by -EquinoX-

okay rule.target is set? what do you mean? how can I set that?

---

Well, I don't know. Reading the code, this is the first time I ever saw rule.target, and right away we're comparing it to something. Who knows what value is stored in there? I'm not sure what's going on here -- it appears that you're deleting the old rule maybe; but there's no guarantee that rule.target is NULL the first time you get to this point. So it's quite possible you're following garbage pointers here.

okay fixed that already, next

I am also assuming this has an error, as if I try to call this it is what creates the seg fault.. can you please tell me why?

Code:

---

rule_t* RuleConstruct(rule_t* rule, rule_t* newrule)
{
  char** dep = (char**) malloc(sizeof(char*) * rule->depSize);
  int h;
  for (h = 0; h < rule->depSize; h++){
    dep[h] = malloc((strlen(rule->dependency[h]) + 1) * sizeof(char));
    strcpy(dep[h], rule->dependency[h]);
  }

  char** bld = (char**) malloc(sizeof(char*) * rule->bldSize);
  int i;
  for (i = 0; i < rule->bldSize; i++){
    dep[i] = malloc((strlen(rule->buildcmd[i]) + 1) * sizeof(char));
    strcpy(dep[i], rule->buildcmd[i]);
  }

  char* target = (char*) malloc((strlen(rule->target) + 1) * sizeof(char));
  strcpy(target, rule->target);
 
  newrule->dependency = dep;
  newrule->buildcmd = bld;
  newrule->bldSize = rule->bldSize;
  newrule->depSize = rule->depSize;
  newrule->target = target;

  return newrule;
}

---

Have you tried reading the "debug guide" on cpwiki?

http://cpwiki.sourceforge.net/Debugging

A segfault should be easy to detect if you have a debugger at all on the machine (I'm guessing this is Linux, so try doing "gdb myprog", then "run", do the steps to make it crash, and "stack" to see where you ended up).

--
Mats

I am planning to use valgrind, but I don't know how so that valgrind can show me the line number in the error

Probably because you call it from here:

Code:

---

for (temp = this->table[hashval]; temp != NULL; temp = temp->next){
    if (strcmp(key, temp->rule->target) == 0){
      RuleDestruct(rule);
      rule_t* newrule;
      temp->rule = RuleConstruct(rule, newrule);     
      return false;
    }

---

And in this code rule is gone, since it was Destructed; but calling Construct without a valid rule in the first parameter leads to bad things.

You'll have to decide what you want to do when you have two rules for the same target, but this ain't it.

even when calling the RuleDestruct alone it gives me the seg fault... so I guess I'll have to construct first and then destruct

I should call RuleDestruct(temp->value) instead of RuleDestruct(rule).. fixed that now..