Memory Error while reading the string

Question

1.00/5 (2 votes)

See more:

VS2012

C++

#include <stdio.h>
#include <stdlib.h>
#include <conio.h>
#include <string.h>
#include <assert.h>

#define TODO //TODO

typedef struct {
	void *elems;
	int elemSize;
	int logLength;
	int allocLength;
} stack;

void stackNew(stack*, int);
void stackPush(stack*, void *elemAddr);
void stackPop(stack*, void *elemAddr);
void stackDispose(stack*);

int main() {
	const char *friends[] = {"Al", "Bob", "Carl"};
	char *name;
	int i, j;

	// Creating the new stack.
	stack stringStack;
	stackNew(&stringStack, sizeof(char *));

	// Pushing strings onto the stack.
	for (i = 0; i < 3; i++) {
		char *copy = _strdup(friends[i]);
		stackPush(&stringStack, ©);
	}

	// Poping the stack elements.
	for (i = 0; i < 3; i++) {
		stackPop(&stringStack, &name);
		printf("%s\n", name);        // error is generated here
		free(name);
	}

	// Disposing off the stack memory.
	stackDispose(&stringStack);

	_getch();
	return EXIT_SUCCESS;
}

void stackNew(stack *stringStack, int elemSize) {
	stringStack -> elemSize = elemSize;
	stringStack -> logLength = 0;
	stringStack -> allocLength = 3;
	stringStack -> elems = malloc((stringStack -> allocLength) * elemSize);
	assert((stringStack -> elems) != NULL);
}

void stackPush(stack *stringStack, void *elemAddr) {
	void *target = (char *) (stringStack -> elems) + 
		(stringStack -> logLength) * (stringStack -> elemSize);
	memcpy(target, elemAddr, stringStack -> elemSize);
	(stringStack -> logLength)++;
}

void stackPop(stack *stringStack, void *elemAddr) {
	void *source = (char *) stringStack -> elems + 
		(stringStack -> logLength) * (stringStack -> elemSize);
	memcpy(elemAddr, source, (stringStack -> elemSize));
	(stringStack -> logLength)--;
}

void stackDispose(stack *stringStack) {
	free(stringStack -> elems);
}

Posted 24-Feb-15 7:01am

Member 11476553

Updated 24-Feb-15 17:47pm

v4

Add a Solution

Comments

ZurdoDev 24-Feb-15 13:12pm

What's the error?

Member 11476553 24-Feb-15 13:13pm

Memory error while reading the string!
It is a run-time error!

ZurdoDev 24-Feb-15 13:14pm

You can't get the exact error?

Member 11476553 24-Feb-15 13:16pm

Unhandled exception at 0x551716B3 (msvcr110d.dll) in Lecture7Example1.exe: 0xC0000005: Access violation reading location 0xFDFDFDFD.

Andreas Gieriet 24-Feb-15 13:52pm

This is not C++. In C++ you would not define your own stack type but use the std::stack class.
Is this an option for you? If not, remove the C++ tag. Your code is pure C.
Cheers
Andi

4 solutions

Solution 1

Learn about the magic numbers on VS C++: see Magic Numbers in Visual C++[^].

You try to be very generic in a language that does not really support this.
You have to employ void pointers and book keeping length of elements, cast forth and back a lot. Just not maintainable.

Your code has several other flaws beside the general ones above.
E.g. Disposing does not deal with disposing the stored elements, etc.

If I had to program a stack in C, I'd start with an int stack.
If I master this, I would go for a char[] (i.e. char*) stack where I had to deal with memory ownership (you have to decide who owns the passed element - do you duplicate internally or do you require the client code to duplicate and the stack frees at disposing the stack, or the stack does not own the element, or any other (more obscure) scheme?)).

Only then I would go to do a general-purpose stack class (if really really really needed!). Then you have to make clear how you can bring the int stack and the char* stack under one hood.

Since you have tagged the question with C++, go and use the std::stack class. You re-invent the wheel here. Only justifiable as an exercise to learn that doing so is waste of time ;-).

Cheers
Andi

Posted 24-Feb-15 8:11am

Andreas Gieriet

Comments

Richard MacCutchan 24-Feb-15 14:52pm

It's an obvious homework assignment.

Solution 2

C++

// Pushing strings onto the stack.
	for (i = 0; i < 3; i++) {
		char *copy = _strdup(friends[i]);
		stackPush(&stringStack, ©);
	}

You are passing the address of the pointer rather than the pointer. It should be

C++

// Pushing strings onto the stack.
	for (i = 0; i < 3; i++) {
		char *copy = _strdup(friends[i]);
		stackPush(&stringStack, copy);
	}

I did not read much after that, as it looks far too messy.

Posted 24-Feb-15 8:51am

Richard MacCutchan

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Mohibur Rashid · Accepted Answer · 2015-02-24T17:37:00

Solution 3

You have lots of ambiguity in your code.
Such as:
On stackNew function you have passed sizeof (char*)
sizeof any pointer will return 4.

you tried to free stringStack but you never allocate it. You have allocated stringStack->elems

in stackPop function you are trying to set value to elemAddr if you give a careful look at no point you have allocated variable name which is passed to stackPop function.

Another issue would be: This not how stack work. You may need to pay more attention at your class and textbook.

Posted 24-Feb-15 17:37pm

Mohibur Rashid

Comments

Member 11476553 24-Feb-15 23:46pm

@Mohibur Rashid, your comment was really helpful

Firstly, I realised that I have allocated stringStack->elems but was trying to free the stringStack, so I have corrected that by editing the statement in stackDispose() function as follows
free(stringStack -> elems);

Secondly, I donot understand the second point that you mentioned "in stackPop function you are trying to set value to elemAddr if you give a careful look at no point you have allocated variable name which is passed to stackPop function". I think I owe you an explanation of what I am trying to do. I created a pointer to point to the same string or (char *) as the perticular element of stringStack and then I free that element of stringStack. I print the poped element using *name. I do this for all the elements of the stringStack and keep pointing the 'name' variable to the current element of stringStack being poped. I hope I made it clear.

Mohibur Rashid 25-Feb-15 0:08am

Ok here is the suggestion,
replace the void* to char* and then debug the code, you will get better view.

Member 11476553 25-Feb-15 0:46am

Can you suggest some good book for generic programming or just some good programming books to become expect in programming.

Mohibur Rashid 25-Feb-15 0:58am

Well, a book or one book will not take you anywhere. You will have to work a lot.

My suggestion is to understand pointer a little more you can read the book "c pointers" by balaguruswamy. Well, my attitude was solving every logical problem with programming. It may help...

Member 11476553 25-Feb-15 1:11am

Thanks Mohibur, that was great help. I solved the problem. Just have a look.

Member 11476553 · Accepted Answer · 2015-02-24T18:55:00

I just made a few changes to the stackPop() function

C++

void stackPop(stack *stringStack, void *elemAddr) {
	void *source = (char *) stringStack -> elems + 
		(stringStack -> logLength - 1) * (stringStack -> elemSize);
	memcpy(elemAddr, source, (stringStack -> elemSize));
	(stringStack -> logLength)--;
}

Notice the statement:

C++

void *source = (char *) stringStack -> elems + (stringStack -> logLength - 1) * (stringStack -> elemSize);

I just had to use (logLength - 1) for obvious reasons, that I failed to notice before.

Memory Error while reading the string

4 solutions

Solution 3

Solution 1

Solution 4

Solution 2

Add your solution here

Preview 0

Existing Members

...or Join us