C gurus! I need an explanation!

[Mucman]

I am just playing around with a queue data type in C. I have a very basic program that inserts 300 't' chars into the queue and spits em back out. First here are the relevant queue functions :

struct qentry
{
struct qentry *next;
char d;
};

struct queue
{
struct qentry *begin, **end;
int size;
};

int qinsert(struct queue *q, char d)
{
if (!q || !(*q->end = (qentry*)malloc(sizeof(struct qentry)))) return 0;
(*q->end)->d = d;
(*q->end)->next = NULL;
q->end = &((*q->end)->next);
q->size++;
return 1;
}

char * qremove(struct queue *q, char *d)
{
struct qentry *tmp;

if (!q || !q->begin) return NULL;
tmp = q->begin;
if (!(q->begin = q->begin->next)) q->end = &q->begin;
*d = tmp->d;
free(tmp);
q->size--;
return d;
}

Here is the main routine that doesn't work :

int main (int argc, char **argv) {
queue* inq;
inq = qinit(NULL);
int i;
char blah = 't';
for (i=0;i<300;i++) {
qinsert(inq,blah);
}

char * temp;

for (i=0;i<300;i++) {
qremove(inq,temp);
printf("%c\n", *temp);
}

return 0;
} /* main */

This gives me a BUS ERROR (core dumped) message when running the program

The following runs just fine... the only difference is that I allocated memory for the temp character pointer

int main (int argc, char **argv) {
queue* inq;
inq = qinit(NULL);
int i;
char blah = 't';
for (i=0;i<300;i++) {
qinsert(inq,blah);
}

char * temp = (char*)malloc(sizeof(char));

for (i=0;i<300;i++) {
qremove(inq,temp);
printf("%c\n", *temp);
}

return 0;
} /* main */

The following also works, but look at what has changed... instead of passing a char to the qinsert function I hardcoded
't' in the argument list! I didn't allocate memory for the temp character pointer and it runs fine!

int main (int argc, char **argv) {
queue* inq;
inq = qinit(NULL);
int i;
for (i=0;i<300;i++) {
qinsert(inq,'t');
}

char * temp;

for (i=0;i<300;i++) {
qremove(inq,temp);
printf("%c\n", *temp);
}

return 0;
} /* main */

Can any C gurus shed a little light on this? I am glad I got it working how I wanted it to, but I don't get why it works!
If you need more code just ask. Yes this is for an assignment, but what I am asking is not in the scope of the assignment
so by explaining this to me you aren't helping me cheat in any way... thanks

 

[BCYL]

In your 'main' function, try declaring your 'char * temp;' at the very beginning of the function (ie. immediately after 'queue* inq').... See if that works for the main that didnt work before...

 

[Mucman]

I'm not sure why that would make a difference... and if it does work, why would it? Could you please explain your rational?

 

[manly]

In ANSI C, all variables have to be declared at the top of a scope. Since you declared temp in the middle of the main() scope, either your compiler doesn't enforce ANSI C or perhaps C99 (the latest standard) allows variable declaraction anywhere in a scope.

At any rate, that's not why your program blows up. Run it through a symbolic debugger and you'll find out exactly where it's screwed up. If I were a better C hacker, I could analyze your code by sight, but I'm not.

 

[Mucman]

Hmm... didn't know that that declaring variables in the beginning is ANSI C. I changed it so that I declared it in the beginning and it still doesn't work. What the heck is a symbolic debugger? Can you give me an example of how one works?

 

[BCYL]

So did it work after you declare your temp variable at the beginning of the main function??? I was thinking since you should declare all variables at the beginning of a function in C, you might be corrupting some memory by declare temp in the middle... This might explain the weird behavior you are seeing...

However, since your program dumped a CORE, it will be easy to debug this using a debugger... What OS are u running? If you are running linux or UNIX, after you program dumped the core, just type:

gdb program_name core

If that didnt work, that means gdb is not installed... then type:

ddd program_name core

gdb is the graphical debugger, but doesnt come on every system... ddd is the text debugger, and should come standard on all linux/unix machines... These commands will help you analysis the core file that was dumped when your program crashed... type 'up' in ddd to go up the stack until u find which statement your program crashed at...

 

[Mucman]

BYCL, no it did not work when changing the declaration of the variable to the beginning. I am doing this on a FreeBSD machine when at home and on a Solaris machine at school. I will try those debugging tips you suggested! I've never done core debugging before, sounds like fun

 

[Nothinman]

gdb is the graphical debugger, but doesnt come on every system... ddd is the text debugger, and should come standard on all linux/unix machines...

Ya got those backwards.

In gdb after the crash you should be able to type 'bt' to see a back trace of what functions were called before the crash.

 

[Mucman]

I know the code isn't nicely formatted because of the forums, but can anyone tell just by looking at the code?

 

[BCYL]

Originally posted by: Nothinman

gdb is the graphical debugger, but doesnt come on every system... ddd is the text debugger, and should come standard on all linux/unix machines...

Ya got those backwards.

In gdb after the crash you should be able to type 'bt' to see a back trace of what functions were called before the crash.

Silly me! Yeah I got those backwards... ddd is the graphical debugger, while gdb is the text-debugger...

 

[manly]

Originally posted by: Mucman
I know the code isn't nicely formatted because of the forums, but can anyone tell just by looking at the code?

You would find the problem quicker tracing the code w/ a symbolic debugger than waiting for an AT geek wannabe programmer to figure it out.

Actually, looking at the code some more, I think the problem as you discovered is pretty obvious.

In the main() method that "doesn't work" you're passing in an uninitialized pointer temp to qremove(). qremove() dereferences that pointer (pointer cardinal sin #1), and your application blows up. Since I'm not much of a C hacker these days, the only way to know for sure is to run your app through gdb (notice I keep repeating myself, for good reason IMHO) and see if this is indeed where your code barfs.

Since you're dealing with discrete characters (as opposed to strings or data structures), malloc isn't really necessary either.

You could instead do:

int c; /* You called this variable temp */
...
qremove(inq, &c);

int & char are relatively interchangeable, and if I'm not mistaken you'll find a lot of standard C library functions that accept character arguments actually are prototyped to take ints (or int pointers). I don't remember if this duality is just a historical relic/mistake or if there's some deeper meaning behind it. Just some extra food for thought.

 

[BCYL]

Originally posted by: manly

Originally posted by: Mucman
I know the code isn't nicely formatted because of the forums, but can anyone tell just by looking at the code?

You would find the problem quicker tracing the code w/ a symbolic debugger than waiting for an AT geek wannabe programmer to figure it out.

Actually, looking at the code some more, I think the problem as you discovered is pretty obvious.

In the main() method that "doesn't work" you're passing in an uninitialized pointer temp to qremove(). qremove() dereferences that pointer (pointer cardinal sin #1), and your application blows up. Since I'm not much of a C hacker these days, the only way to know for sure is to run your app through gdb (notice I keep repeating myself, for good reason IMHO) and see if this is indeed where your code barfs.

Since you're dealing with discrete characters (as opposed to strings or data structures), malloc isn't really necessary either.

You could instead do:

int c; /* You called this variable temp */
...
qremove(inq, &c);

int & char are relatively interchangeable, and if I'm not mistaken you'll find a lot of standard C library functions that accept character arguments actually are prototyped to take ints (or int pointers). I don't remember if this duality is just a historical relic/mistake or if there's some deeper meaning behind it. Just some extra food for thought.

I think that's it... You are trying to store a char value in the temp variable you declared in main(), but you only declared it as a pointer to char (therefore there's not enough room to store the value)... instead, you should declare it as 'char temp' and pass in the address of temp into qremove (ie. &temp)...

BTW, I am also located in Vancouver... What class is this for?

 

[Mucman]

manly - Hehe, don't worry I will learn gdb, unfortunately I have to get crackin on the rest of the assignment first... I totally forgot about passing a reference! That's the way I am going to do it now... I am still perplexed about one thing though. Look at my last code sample; That piece of code runs fine! I still used 'char * temp' but in the insert I passed 't' statically instead of passing a char to the function. btw, I brought this to the geeks of AT because I won't be seeing my prof until tommorrow I am going to present this to him and see what he says.

BCYL - What I've shown has nothing really to do with the assignment... We are writing a C program using the pthread.h library. They communicate via 2 finite queues which is the portion that I am working on now. It's for the course CMTP300 - Operating Systems I at SFU. The prof is Dr. Brad Bart who is very good, but also very hard... I am thoroughly enjoying this course right now

 

[BCYL]

Originally posted by: Mucman
manly - Hehe, don't worry I will learn gdb, unfortunately I have to get crackin on the rest of the assignment first... I totally forgot about passing a reference! That's the way I am going to do it now... I am still perplexed about one thing though. Look at my last code sample; That piece of code runs fine! I still used 'char * temp' but in the insert I passed 't' statically instead of passing a char to the function. btw, I brought this to the geeks of AT because I won't be seeing my prof until tommorrow I am going to present this to him and see what he says.

BCYL - What I've shown has nothing really to do with the assignment... We are writing a C program using the pthread.h library. They communicate via 2 finite queues which is the portion that I am working on now. It's for the course CMTP300 - Operating Systems I at SFU. The prof is Dr. Brad Bart who is very good, but also very hard... I am thoroughly enjoying this course right now

pthreads... hehe

I took an equivalent course at UBC around 2 years ago (intro OS course)... we used the pthreads library as well... I really liked that course...

In the advanced OS course at UBC we start to do some kernel hacking... if you like this one you should definitely take that one as well...

 

[Mucman]

Originally posted by: BCYL

Originally posted by: Mucman
manly - Hehe, don't worry I will learn gdb, unfortunately I have to get crackin on the rest of the assignment first... I totally forgot about passing a reference! That's the way I am going to do it now... I am still perplexed about one thing though. Look at my last code sample; That piece of code runs fine! I still used 'char * temp' but in the insert I passed 't' statically instead of passing a char to the function. btw, I brought this to the geeks of AT because I won't be seeing my prof until tommorrow I am going to present this to him and see what he says.

BCYL - What I've shown has nothing really to do with the assignment... We are writing a C program using the pthread.h library. They communicate via 2 finite queues which is the portion that I am working on now. It's for the course CMTP300 - Operating Systems I at SFU. The prof is Dr. Brad Bart who is very good, but also very hard... I am thoroughly enjoying this course right now

pthreads... hehe

I took an equivalent course at UBC around 2 years ago (intro OS course)... we used the pthreads library as well... I really liked that course...

In the advanced OS course at UBC we start to do some kernel hacking... if you like this one you should definitely take that one as well...

UBC! I am sooo sorry





Yeah, I will definitely be taking the next one. I've been doing so much high level programming that I am sucking at C right now so 50% of my assignment
is spent re-learning C.

 

[m0ti]

Couple of things:

1) It isn't considered good practice to not release memory at the end of your program. Make a qdestroy function, and release all of your memory (go down the list freeing the qentries as you go).

2) You're qinsert function doesn't work at all.

take a look at the first line: if (!q || !(*q->end = (qentry*)malloc(sizeof(struct qentry)))) return 0;

note that you've just changed the pointer to the end of the q. (and you declared end to be struct qentry**) therefore you may be leaking more memory. You should be doing a (qentry**)malloc(sizeof(struct qentry *)).

In any case you then try to do:

(*q->end)->d = d;
(*q->end)->next = NULL;
q->end = &((*q->end)->next);

the last line translates to: q->end = &(NULL) = ????? Seg fault.

You should really be handling this in a different way. Use a temp variable to create your new qentry, then add that to the end of your list (end->next), and then move end to point temp.

If you don't want to use a debugger (which you don't need for a simple program like this), just include some printf()'s. Things like printf("Entering qinsert\n"); ... printf("Exiting qinsert\n"); You can then easily see where your programming is going down.

Hope that this helps.

 

[Mucman]

m0ti - Yup it is good practice to clean up isn't it? All that JAVA has got me spoiled!

I got the queue from this website. I understand the implementation that you are describing and I do like it better the way you described. I need to
alter the queue anyways because it's not suppose to get any larger than 256 bytes, so I may do that in the course of my alterations. Thanks for the tips!

 

[manly]

IMO m0ti is one of the non-wannabe programmer geeks on AT.

However, I have to disagree with his comment that this problem is too simple for a symbolic debugger. From what I've seen, the vast majority of coders use printf's for debugging code; that behavior really should be discouraged rather than condoned or even recommended. IMHO you'll become a better coder if you drop into a debugger any chance you have, rather than sprinkling your code with printf's, even for some simple problems such as this. This opinion partly comes from RMS, so it's more of a personal bias than some cardinal rule.

As for your 3rd main() implementation, I don't see any significance in passing in the literal 't' as an argument to qinsert. C passes arguments by value anyway, so if it works, it's due to some other subtle behavior elsewhere. Miss Java already?

 

[singh]

Ahh.. it's so easy to appreciate C++ when looking at horrible-looking C Code

 

[Nothinman]

From what I've seen, the vast majority of coders use printf's for debugging code; that behavior really should be discouraged rather than condoned or even recommended. IMHO you'll become a better coder if you drop into a debugger any chance you have, rather than sprinkling your code with printf's, even for some simple problems such as this.

debuggers are good and should be used whenver possible, but printfs aren't evil. I wish more programs came with a 'debug' mode that printed verbosly what it was doing so that I, as the application user, could figure out the problem without the source and running the program through gdb.

 

[BCYL]

Originally posted by: manly
IMO m0ti is one of the non-wannabe programmer geeks on AT.

However, I have to disagree with his comment that this problem is too simple for a symbolic debugger. From what I've seen, the vast majority of coders use printf's for debugging code; that behavior really should be discouraged rather than condoned or even recommended. IMHO you'll become a better coder if you drop into a debugger any chance you have, rather than sprinkling your code with printf's, even for some simple problems such as this. This opinion partly comes from RMS, so it's more of a personal bias than some cardinal rule.

As for your 3rd main() implementation, I don't see any significance in passing in the literal 't' as an argument to qinsert. C passes arguments by value anyway, so if it works, it's due to some other subtle behavior elsewhere. Miss Java already?

When you are writing multi-threaded code and linux doesnt do multi-threaded core dumps, printf's can be very useful at debugging...

 

[Mucman]

Is my code that ugly singh? The indentations were lost when I pasted them in the forums...

manly, I actually prefer C over JAVA. I definitely got more done with JAVA, but I am having more fun doing this

I now have a multithreaded file filter right now! It runs and each thread prints when it is running so I can watch the execution... it's not threadsafe right now so i am
reading up on mutex to get the 3 threads sharing the 2 queues... If anyone is interested I will post my program after the due date. The prof doesn't take any late assignments
so if there are any SFU lurkers, they will be out of luck if they want to copy

 

[m0ti]

IMO m0ti is one of the non-wannabe programmer geeks on AT.

Thanks. Considering the number of hours I've put in programming that's a relief. Of course, the same goes back to you, manley.

And btw, I very much so agree about printf's vs. a proper debugger. There really is no comparison. I didn't say that he SHOULD use printf's, I said that he COULD.

In any case, there realy is no compromising creating proper tests (JUnit for me, though it's easy enough to write C equivalents (for the tests, not the entire framework. I know Erich Gamma and Kent Beck spent enough time on that), and having good debugging tools.

As for multi-threaded programs (from experience) they are practically impossible to debug with printf's above a certain amount of complexity. You at the very least need a debugger which will allow you to hitch a ride along with your threads (one at a time as they are run), or at least show you where deadlock took place. Of course, there do exist great tools which show you all the thread's history and more so it's much easier.

 

[Mucman]

Wow! Major props for anyone who does C coding for a living . I was getting strange behavior (segmentation faults) frequently and it's because
I don't have a clue how to manage memory. I was passing a pointer of a struct containing all the shared variables between the 3 threads. I was failing
to do it properly so at the last minute I made all the variables global by putting them in a globals.h file and using extern.

Anyways, I got the majority of it done... I probably spent way too much time on this assignment mostly because of bad programming practices. Can anyone reccomend some sites on how to handle larger than 1 source file programs? Too many examples on the net have 1 source file which makes everything simple. My program had at leat 8 or 9, but I am not sure if I should have cut it down to 4 or made it up to 12. Also a few links on good debugging techniques would have helped... I spent 70% debugging and the rest actually programming.

If anyone is interested in my source just PM me.