Why am I getting a seg fault with this code? Should be simple?

The problem is in the last two lines of Push.

Up vote 1 down vote favorite share g+ share fb share tw.

I'm at a complete loss. I feel like there must be some glaring, stupidly easy mistake here, and my eyes are just too tired to see it. I'd really appreciate any help.

When I run testProb2, the program prints "Pushing:" and then on the next line, "Segmentation fault". I find this really weird, because the next thing after printf("Pushing:\n"); is another call to printf with just a static argument being passed, nothing dynamic that could be doing weird crazy things in some other method, and yet "1 " does not get printed. I put the call to printf for 1 and 2 in there just as a test, because I initially thought that the problem might be in my first for loop, which is commented out right now, but that's not it.

As I said, I believe the problem to be in testProb2. C, but I included stackli. H and stackli.

C below it just in case. I'm compiling this with "gcc -ansi stackli. C testProb2.

C -o testProb2". /* testProb2 * * Demonstrates a stack implementation that allocates a number of nodes at creation of the stack * rather than on each call to Push. All methods are O(1) except for GrowFreeList, which is O(n), * and CreateStack, which is O(n) because it calls GrowFreeList, and possibly Push, which will be * O(1) when called while there are empty nodes, but O(n) when called if there are not empty nodes.

*/ #include "stackli. H" #include int main(void) { Stack S; int i; S = CreateStack(8); printf("Pushing:\n"); printf("1 "); Push(1, S); printf("2\n"); Push(2, S); /* for (i = 0; I struct StackRecord { PtrToNode ThisStack; PtrToNode FreeNodes; int Size; }; struct Node { ElementType Element; PtrToNode Next; }; /* O(n) instead of O(1) because it calls GrowFreeList which is O(n) */ Stack CreateStack(int initialSize) { Stack S; S = malloc( sizeof( struct StackRecord ) ); S->ThisStack = NULL; S->FreeNodes = NULL; S->Size = initialSize; GrowFreeList(S); return S; } /* O(n) function */ void GrowFreeList(Stack S) { int i; PtrToNode temp; for (i = 0; I Size; i++) { temp = malloc( sizeof( struct Node) ); if (temp == NULL) FatalError("Out of space! "); temp->Next = S->FreeNodes; S->FreeNodes = temp; } S->Size = S->Size * 2; } c homework segmentation-fault link|improve this question edited Nov 8 '11 at 3:01 asked Nov 3 '11 at 3:17Erika E467 100% accept rate.

It often happens, just add \n at the end of the other printf and it will generally appear. Anyway it doesn't solve the problem. – lc2817 Nov 3 '11 at 3:19 1 The problem could occur after the second printf.

The standard output is by default line buffered, which means output is only sent when you send a newline character (and a few other cases which flush all streams) unless you explicitly flush it. This means your "1 " output is most likely buffered when the error occurs in the Push function, so it is never printed. – ughoavgfhw Nov 3 '11 at 3:24 1 To further explain @lc2817's comment: Output if buffered.

Without a \n in your printf("1 "); statement it isn't being flushed before the segfault. – Brian Roach Nov 3 '11 at 3:24 2 It's a stack implemented with a linked list, from my point of view, it doesn't make sense to preallocate nodes, just allocate them when you need. This will simplify your code a lot and help you understand the error.

– Salvatore Previti Nov 3 '11 at 3:38.

The problem is in the last two lines of Push: void Push( ElementType X, Stack S ) { PtrToNode temp; ... temp->Next = S->ThisStack->Next; S->ThisStack = temp; } When you first call Push, the ThisStack field is null. When you try to dereference it to access its Next field, you are getting a segfault. However, since the top of the stack is in ThisStack, not ThisStack->next, fixing that problem will get rid of the segfault.

Void Push( ElementType X, Stack S ) { PtrToNode temp; ... temp->Next = S->ThisStack; S->ThisStack = temp; } You make the same mistake in Pop, which would cause you to entirely skip the first element. The assignment to temp should be like this: temp = S->ThisStack; Finally, your Size field will always be wrong. It appears that GrowFreeList is supposed to double the size of the stack when it is called, but when you call it from CreateStack, your stack's real size is 0 although the Size field is 8 (in your example).

The result is that the stack contains 8 free nodes, but its Size field is 16. In fact, the Size field will always be larger than the actual size by the initial size. This doesn't cause any problems now, since it is only used to determine how many to add, but the fix is simple: reset the Size field after calling GrowFreeList from CreateStack: Stack CreateStack(int initialSize) { Stack S; ... S->Size = initialSize; GrowFreeList(S); S->Size = initialSize; // Reset size, since GrowFreeList changed it return S; }.

Thanks, wow, your answer was so comprehensive, and I see that you're right about the Size field, although I believe your fix to be incorrect. I changed my implementation of GrowFreeList instead, but thanks again, I wouldn't have noticed that myself. – Erika E Nov 3 '11 at 7:29.

Okey doke ... here we go: You initialize your Stack S with CreateStack(8). This sets ThisStack to NULL inside the struct. Then the first thing you so is call Push(1,S) in which you do ... temp->Next = S->ThisStack->Next; Kaboom.

You just dereferenced a NULL pointer. Ideally you want to get familiar with using the debugger (gdb). This will let you step through the execution of your code see exactly where it's blowing up.

Kaboom indeed! I wish I could give two right answers. And I will learn about gdb, thank you for the tip.

– Erika E Nov 3 '11 at 7:14.

As others are saying, you should use fprintf(stderr,...) to print debug messages. You can use gdb to see where the problem is and see what's going on in memory. Add the flag -g when compiling to help with debugging in gdb.

This is the output gdb gave me: (gdb) r Starting program: .../test Pushing: 1 2 2 Program received signal SIGSEGV, Segmentation fault. 0x0000000000400873 in Push (X=1, S=0x601010) at stackli. C:81 81 temp->Next = S->ThisStack->Next; (gdb) bt #0 0x0000000000400873 in Push (X=1, S=0x601010) at stackli.

C:81 #1 0x0000000000400627 in main () at testprob2. C:20 (gdb) print temp $1 = (PtrToNode) 0x601110 (gdb) print S->ThisStack $2 = (PtrToNode) 0x0 The error is occurring in void Push( ElementType X, Stack S ). As you can see when we print out S->ThisStack we get 0x0, ie.

S->ThisStack is pointing to NULL. When you dereference S->ThisStack to get to S->ThisStack->Next you'll get a segmentation fault. You could add a check for if (S->ThisStack == NULL).

I'm not really sure with the way you're going about creating a stack structure. Also note, this is only one problem with your code, there are potentially (and most likely) a lot more problems, however you should be able to narrow them down with gdb using the backtrace (bt) command, and printing to stderr as opposed to stdout so that your output is not buffered. I don't want to do your homework for you.

1 Typo: stdout, not stdin. – Daniel Fischer Nov 3 '11 at 3:44 @DanielFischer Thanks for that, fingers have a mind of their own sometimes. – AusCBloke Nov 3 '11 at 3:46.

The comments about flushing are correct. The problem is in the second to last line of Push. You never initialized ThisStack, so when you try to access its next element you get a segfault.

Incidentally, you size is off because you double it after filling it for the first time. Finally, you should try to avoid typedefing away pointers. It's too easy to forget what's a pointer and what's not.

Make stack the full struct and declare pointers to it.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.