Iterating over elements in a deque results in garbage.

T

Travis L Spencer

Hey,

I am trying to insert a bunch of Token objects into a collection
called tokens of type deque<Token>. The problem is that the output of
Tokens::print is just junk. For instance, the call at the end of main
produces this output:

Value: fsdf
Width: 0

Value: dfwf
Width: 0

Value: sdfasd
Width: 0

The call to cout's insertion operation on the line after tokens.print
produces this output:

Value: test3
Width: 5

I can't figure it out. I suspect that it has something with variables
going out of scope, but I could use some help finding the problem.
Here is a simplified example:

#include <deque>
#include <iostream>
#include <cstring>
#include <cassert>

using namespace std;

class Token
{
const Token& operator=(const Token& r);
char value[255];
int width;

public:

Token(void) : width(0) { /*EMPTY*/ }

Token(const Token& t) {
strcpy(value, t.value);
width = t.width;
}

void append(const char c) {
value[width++] = c;
}

friend ostream& operator<<(ostream& os, const Token& token) {
os << "Value: " << token.value << endl
<< "Width: " << token.width << endl;

return os;
}
};

class Tokens
{
deque<Token> tokens;
Tokens(const Tokens&);

public:

Tokens(void) { /*EMPTY*/ }

Token* begin(void) {
return &*tokens.begin();
};

Token* end(void) {
return &*tokens.end();
};

void addNew(void) {
Token newToken;
this->tokens.push_back(newToken);
};

void Tokens::print(std::eek:stream& output = cout)
{
std::deque<Token>::iterator itr;

for (itr = tokens.begin() ; itr != tokens.end() ; itr++ )
output << " " << *itr << std::endl;
}
};

int main(int c, int v)
{
char* test[] = {"Test", "Test2", "test3"};
Tokens tokens;

for (int i = 0; i < 3; i++)
{
tokens.addNew();

for (int j = 0; j < strlen(test); j++)
tokens.end()->append(test[j]);
}

tokens.print(); // This prints all of the tokens but they are rubbish.
cout << *tokens.end(); // This prints a test3 that isn't just junk.

return 0;
}

Thanks in advance.

--

Regards,

Travis Spencer
Portland, OR. USA
 
I

Ivan Vecerina

Travis L Spencer said:
I am trying to insert a bunch of Token objects into a collection
called tokens of type deque<Token>. The problem is that the output of
Tokens::print is just junk.
Hi Travis... Just taking a quick read through:
Token(const Token& t) {
strcpy(value, t.value);
width = t.width;
}
Actually this class would be ok with the default copy
constructor (which would do a byte-wise copy of the array).
void append(const char c) {
value[width++] = c;
}
Some overflow checking would be nice.

class Tokens ....
Token* end(void) {
return &*tokens.end();
}; ....
for (int j = 0; j < strlen(test); j++)
tokens.end()->append(test[j]);

That's the problem: container.end() returns a one-past-the-end
element which shall not be dereferenced (e.g. like the a[5]
element of the variable int a[5] ).
What you want to use is the back() member function of
std::deque (or any other sequence container), which returns a
valid reference to the last element in a non-empty collection:
Token& back(void) {
return tokens.back();
};
....
for (int j = 0; j < strlen(test); j++)
tokens.back().append(test[j]);


Cheers,
Ivan
 
P

Peter Koch Larsen

Travis L Spencer said:
Hey,

I am trying to insert a bunch of Token objects into a collection
called tokens of type deque<Token>. The problem is that the output of
Tokens::print is just junk. For instance, the call at the end of main
produces this output:

Value: fsdf
Width: 0

Value: dfwf
Width: 0

Value: sdfasd
Width: 0

The call to cout's insertion operation on the line after tokens.print
produces this output:

Value: test3
Width: 5

I can't figure it out. I suspect that it has something with variables
going out of scope, but I could use some help finding the problem.
Here is a simplified example:

#include <deque>
#include <iostream>
#include <cstring>
#include <cassert>

using namespace std;

class Token
{
const Token& operator=(const Token& r);
char value[255];

Looking at the rest of the code, this looks like a null-terminated C-string.
Instead, I'd recommend you use std::string - a much safer choice.
int width;

public:

Token(void) : width(0) { /*EMPTY*/ }

Here value is undefined - you will not be able to print it.
Have value[0] = 0;
Token(const Token& t) {
strcpy(value, t.value);
width = t.width;
}

void append(const char c) {
value[width++] = c;

Once again you destroy the null terminator needed if you are to print your
values out.

}

friend ostream& operator<<(ostream& os, const Token& token) {
os << "Value: " << token.value << endl
<< "Width: " << token.width << endl;

return os;
}
};

class Tokens
{
deque<Token> tokens;
Tokens(const Tokens&);

public:

Tokens(void) { /*EMPTY*/ }

Token* begin(void) {
return &*tokens.begin();
};

Token* end(void) {
return &*tokens.end();
};

void addNew(void) {
Token newToken;
this->tokens.push_back(newToken);
};

void Tokens::print(std::eek:stream& output = cout)
{
std::deque<Token>::iterator itr;

for (itr = tokens.begin() ; itr != tokens.end() ; itr++ )
output << " " << *itr << std::endl;
}
};

int main(int c, int v)
{
char* test[] = {"Test", "Test2", "test3"};
Tokens tokens;

for (int i = 0; i < 3; i++)
{
tokens.addNew();

for (int j = 0; j < strlen(test); j++)
tokens.end()->append(test[j]);

This loop looks horrible, but the worst thing is that you dereference end().
End points past any values, and you are not allowed to modify it.
}

tokens.print(); // This prints all of the tokens but they are rubbish.
cout << *tokens.end(); // This prints a test3 that isn't just junk.

return 0;
}

Thanks in advance.

--

Regards,

Travis Spencer
Portland, OR. USA

/Peter
 
T

Travis L Spencer

Token(void) : width(0) { /*EMPTY*/ }

Here value is undefined - you will not be able to print it.
Have value[0] = 0;

OK. I rewrote is like so:

Token(void) : width(0), value(0) { /*EMPTY*/ }
Token(const Token& t) {
strcpy(value, t.value);
width = t.width;
}

void append(const char c) {
value[width++] = c;

Once again you destroy the null terminator needed if you are to print your
values out.

Shoot. I changed the declaration to value[MAX_TOKEN_WIDTH + 1] and in
all constructors but the default one, I called bzero to insure that it
is zeroed out. I also made sure that I never overflow value.

Token {

char value[MAX_TOKEN_WIDTH + sizeof('\0')];
....

public:

Token(int w) : width(w)
{
bzero(value, MAX_TOKEN_WIDTH + sizeof('\0'))
}

void append(const char c)
{
if (width < MAX_TOKEN_WIDTH)
{
value[width++] = c; //
assert(value[width] == '\0');
}
}
....
};
int main(int c, int v)
{
char* test[] = {"Test", "Test2", "test3"};
Tokens tokens;

for (int i = 0; i < 3; i++)
{
tokens.addNew();

for (int j = 0; j < strlen(test); j++)
tokens.end()->append(test[j]);

This loop looks horrible, but the worst thing is that you dereference end().
End points past any values, and you are not allowed to modify it.


Ya, the idea was just to make a stripped down demonstration. It isn't
what I'm actually using to populate the deque of tokens. But you're
right that the end function was the source of my problems. Thanks for
catching that.

--

Regards,

Travis Spencer
Portland, OR. USA
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top