4 ms·
Let me try (I do not often write production C code) 1. You need to malloc 3 words, and you're only allocating one (you want sizeof(Abc)) 2. Abc in the struct w
by peterhunt 15y ago
Let me try (I do not often write production C code)
1. You need to malloc 3 words, and you're only allocating one (you want sizeof(Abc))
2. Abc in the struct will not resolve (change to struct _Abc and the struct name to _Abc)
3. You should cast the result of malloc/check for failure
4. This is bad memory management practice, I think.
- ori_b 15y ago1) Right. 2) No, 'struct Foo' is in a different namespace from the typedef'ed name. 'typedef struct Foo Foo' is perfectly valid. 3) Yes, absolutely. 4) Not sure what you mean. Here's a hint: You'll likely miss one of them unless you realize what the algorithm is trying to do. It's an algorithmic error. The other one is findable from looking at the code without context, but the correct fix isn't entirely obvious without realizing the way the code works. Pretty good, though :)
- 2arrs2ells 15y agoAbc is a node in a doubly linked list. I think function foo is supposed to insert a new node (n) into the list before node p, but it's not done properly (while n points to p and p points to n, n does not point back to the item before p).
- ori_b 15y agoI was going for foo as 'dlist_prepend()', not dlist_insert(), but figuring out that it's putting a node into a list is good enough for me. (Edit: to me, 'prepend' implies that there are no nodes before the head of the list you're prepending to) That should be enough for you to figure the last 2 errors, in any case.
- 2arrs2ells 15y agoPrepend is probably a better way of expressing "insert before" :P n->p = p->p I don't see the last one - unless foo should be returning n?
- ori_b 15y agoYes, it should be returning `n`. And the final one I was thinking of was a segfault on an empty list prepend(NULL, &something); would die if you don't do; if (p != NULL) p->p = n; I think, this would be easier in a larger problem where you had context that gave you usage examples. I probably wouldn't ask someone to debug a function floating an a vacuum like I did here. I'd use a trivial but full working program. I don't think I'd expect everyone to get every bug without a little bit of prodding in the right direction. Being able to step through the code and show what it's doing to the data structures would be a first step that would make me happy. Recognizing that the data structure looked like it's a linked list, and finding and and fixing a couple of the bugs, and I'd be very happy.
- rayval 15y agoFunction should return n rather than p.
- gsg 15y agoCasting the result of malloc isn't a good idea in C because it can hide the lack of a prototype if you forget to #include <stdlib.h>. It's also a redundant mention of the type name in question, making for more work / noisy diffs under code change. Unfortunately C++ requires the cast.