Sign in to follow this  

Having trouble with vectored structures

This topic is 3809 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Maybe it's just me, but I can't seem to get this to work. I'm trying to read structures from a file, while storing them in a vector, as I only know how many structures there are after reading the header. This is how I read in the structures;
where 'resc_list' is a vector, declared as 'std::vector<Resource> resc_list;'

for(uint32_t i=0;i<resource_count;++i)
{
   Resource r;
   in >> r.file_offset;
   in >> r.resource_size;
   in >> r.resource_name;

   resc_list.push_back(r);
}

However, I find that when I come to try to read anything from the vector, it's completely empty! Any ideas what might be wrong?

Share this post


Link to post
Share on other sites
- Have you checked the value of resource_count?
- Have you checked the contents of the file?
- Are you opening the file in a way that's compatible with how you intend to interpret its contents (i.e. you usually want to use text mode when you read with operator>>, and binary mode when you read with .read())?
- How are you checking the contents of the vector? Are you sure the loading code has already run when you do this check?
- Are you communicating (passing) the vector between functions? If so, how?

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
- Have you checked the value of resource_count?
- Have you checked the contents of the file?
- Are you opening the file in a way that's compatible with how you intend to interpret its contents (i.e. you usually want to use text mode when you read with operator>>, and binary mode when you read with .read())?
- How are you checking the contents of the vector? Are you sure the loading code has already run when you do this check?
- Are you communicating (passing) the vector between functions? If so, how?
In defense of my admittedly more brief reply, I'd like to mention that answering the first question should lead the OP naturally to the others listed (which is why I left it at that).

For the benefit of the OP I'll elaborate a bit. First of all, simply checking the value of resource_count (e.g. in the debugger, by way of debug output, etc.) should tell you something about whether the code is being executed in the first place. If at that point resource_count is found to be zero, it's probably a good guess that the file doesn't contain what you think it does, you're interpreting its contents incorrectly, or you're computing the value of resource_count incorrectly. If resource_count is not zero, resc_list.size() must, for all practical purposes, return a non-zero value once the loop has terminated. If you're finding that it is returning zero, then you're checking it in the wrong place, passing the vector to the function incorrectly, or something of that sort.

@The OP: There are a couple of useful lessons to be learned here. First, although it's good not to overwhelm us with code when posting to the forums, it's usually best to provide a little more context than in your example (what are the function arguments? how are they passed? are there global variables involved? from where is the code invoked? etc.). Second, the process of which I gave an example earlier in this post is called 'debugging', and is a skill that is every bit as important to a programmer as being able to write the code in the first place. As you develop this skill, you'll find that these sorts of problems are, in most cases, fairly straightforward to solve.

Share this post


Link to post
Share on other sites
Yeah, I did check both the vectors size, and the value of resource_count, showing that resource_count was very large, while the size of the vector was 0. The snippet of code I showed, is part of the Load function of a class.

The class header;

class BIGFile{
protected:
struct Resource{
uint32_t file_offset;
uint32_t resource_size;
std::string resource_name;
};

uint32_t resource_count;
std::vector<Resource> resc_list;

std::string file;
std::string dest;

std::ifstream in;

public:
BIGFile(std::string file,std::string dest):file(file),dest(dest){}
~BIGFile();
void Load();//loads the file with the given name
uint32_t GetCount(){return resource_count;}
uint32_t GetVecSize(){return resc_list.size();}
std::string GetNames();//returns a string of the filenames, seperated by spaces
void Extract(std::string,std::string);//extracts the named resource to the given location
};




and the whole Load function;

void BIGFile::Load()
{
in.open(file.c_str());
if(in.is_open())
{
char id[4];
in.get(id,4);
if(std::string("BIG4").compare(id)==0)
{
int x;
in >> x;
in >> resource_count;
in >> x;

for(uint32_t i=0;i<resource_count;++i)
{
Resource r;
in >> r.file_offset;
in >> r.resource_size;
in >> r.resource_name;

resc_list.push_back(r);
}
}
}
}




As you can probably see, it's a class for reading Command & Conquer archive files, and the Load function opens the file with the name that the class is constructed with.

EDIT: I've tried changing the original snippet to use in.read() and a string buffer (sbuff), as follows;

for(uint32_t i=0;i<resource_count.u;++i)
{
Resource r;
in.read(r.file_offset.c,sizeof(uint32_t));
in.read(r.resource_size.c,sizeof(uint32_t));
in.get(sbuff,'\0');
r.resource_name = sbuff.str();

resc_list.push_back(r);
}



but still no luck.

[Edited by - webwraith on July 13, 2007 1:47:48 PM]

Share this post


Link to post
Share on other sites
You said you checked the value of resource_count... did you actually step through the loop in the debugger to verify that push_back was being called over and over again?

Your code looks fine as it is, but there must be something else going on that could only be determined by you stepping through with a debugger, or by posting more code surrounding the loop.

Share this post


Link to post
Share on other sites
There are several issues at play here, but for now I just have one question: Are you positive that the line:
in >> resource_count;
Is being executed, and if so, how do you know that this is the case?

Share this post


Link to post
Share on other sites
I'm not conversant with the MingW debugger, but when I run the Debug command from the IDE(Code::Blocks) it runs through the program, then exits, saying there's nothing wrong.

OK, I tried setting the resource counter to 0, and it came through at the end as 0, I'm going to go check that file, now...

EDIT: OK, just found out that std::string::compare() isn't recognizing the "BIG4" at the start of the file. It's in the file (I just checked), so am I wrong in putting it like this;

if(std::string("BIG4").compare(id)==0)


Where id is now declared
char id[5];

Share this post


Link to post
Share on other sites
My advice would be to get familiar with a debugger. You won't be very productive if you have to make random guesses as to what's wrong, or always resort to "printf" debugging.

Stepping through your code with a debugger will probably tell you the problem in a few seconds.

Share this post


Link to post
Share on other sites
Quote:
Original post by webwraith
EDIT: OK, just found out that std::string::compare() isn't recognizing the "BIG4" at the start of the file. It's in the file (I just checked), so am I wrong in putting it like this;

if(std::string("BIG4").compare(id)==0)


Where id is now declared
char id[5];


You keep changing your posts :-)
When I hit reply, it was "char id[4]", and now its "char id[5]" :-)

Well, that's an improvement. Are you sure it's being null-terminated?

Share this post


Link to post
Share on other sites
Quick way to isolate the problem:


for(uint32_t i=0;i&lt; 5; ++i)
{
Resource r;
in.read(r.file_offset.c,sizeof(uint32_t));
in.read(r.resource_size.c,sizeof(uint32_t));
in.get(sbuff,'\0');
r.resource_name = sbuff.str();

resc_list.push_back(r);
}




Just for testing, change the loop for i < 5.

This will tell you weather or not r has data in it, or if resource.u is a correct number (1 or higher)

Share this post


Link to post
Share on other sites
Woohoo! I got it to work!! here's the final function;

void BIGFile::Load()
{
in.open(file.c_str());
if(in.is_open())
{
std::cout << "file name: " << file << std::endl;
BIG4Header b4h={0};
in.read((char*)&b4h,sizeof(BIG4Header));

char_2_ui32 cu32;
cu32.u = b4h.resource_count;
char temp_ch;
temp_ch = cu32.c[0];
cu32.c[0] = cu32.c[3];
cu32.c[3] = temp_ch;
temp_ch = cu32.c[1];
cu32.c[1] = cu32.c[2];
cu32.c[2] = temp_ch;
resource_count=cu32.u;


if(b4h.id[0]=='B' && b4h.id[1]=='I' && b4h.id[2]=='G' && b4h.id[3]=='4')
{
std::cout << "file type: " << b4h.id[0] << b4h.id[1] << b4h.id[2] << b4h.id[3] << std::endl;
std::cout << "number of resources in file: " << resource_count <<std::endl;

Resource r;
char_2_ui32 c232;

for(uint32_t i=0;i<resource_count;++i)
{
std::stringbuf sbuff;
in.read(c232.c,sizeof(std::streampos));
r.file_offset=c232.u;
std::cout << "file offset: " << r.file_offset <<std::endl;

in.read(c232.c,sizeof(uint32_t));
r.resource_size=c232.u;
std::cout << "resource size: " << r.resource_size<<std::endl;

in.get(sbuff,'\0');
r.resource_name = sbuff.str();
std::cout << "resource name: " << r.resource_name << std::endl << std::endl;

resc_list.push_back(r);
}
}
else
{
std::cout << "File type not recognized: " << b4h.id[0] << b4h.id[1] << b4h.id[2] << b4h.id[3] << std::endl;
}
}
else
{
std::cout << "Couldn't open file: " << file << std::endl;
}
}
[\source]
It [i]is[/i] quite a bit longer, but it now has slight error checking (yes, the so-called "printf" error checking :P ) and produces a pretty output XD

Note a new header;
[code]
struct BIG4Header{
char id[4];//must contain 'BIG4'
uint32_t x1;
uint32_t resource_count;
uint32_t x2;
};[/code]

Share this post


Link to post
Share on other sites
Quote:
Original post by webwraith
I'm not conversant with the MingW debugger

Well, there's your problem then. Trying to debug without a debugger is a waste of time. Take the time to learn how to use your debugger.

Share this post


Link to post
Share on other sites
Yeah, mixing .read() operations with operator>> is a pretty good sign of doing something wrong. The former implies unformatted, binary data, while the latter implies formatted, "text" (i.e. human-readable) data. It's unusual for a file to mix those.

That said:

- It looks like you're using some kind of union to reinterpret integers as byte arrays. Don't do that. That's what reinterpret_cast is for.

- You don't need to initialize the header before you read into it, because the reading-in is going to initialize it for you. Unless you're worried that file reading will fail *and* the uninitialized header happens to contain "BIG4" in the right spot - a valid concern, but if the file reading fails, everything's going to abort *anyway*.

- Use a helper function to read the integers, since you do it often. I have a templated one that I reproduce commonly in forum posts :/

- A bigger, design problem: Don't write 'load' functions. That work properly belongs in the constructor; that's what constructors are for. Also, don't hold on to the stream object within the BIGFile (you don't need it after you're done loading; or at least, you can reopen it via the saved file name) nor the resource_count or associated getter (it's redundant). And what's the 'dest' currently for?

- Oh, and don't make things 'protected' without a good reason. Private is private. "protected" exists for the sake of derived classes. Why would you have derived classes of BIGFile?

- Prefer to pass strings (and other class instances) by const reference.

- Are you *really* sure that you need to endian-swap the resource_count, but NOT any of the other ints in the file?

- Don't use std::string::compare() in "normal" code. The class is defined to make comparison operators work the way you expect, and it's much more readable that way. .compare() exists for when you have to do 3-way comparison and you have a slick way of using the resulting return value.


class BIGFile {
struct Resource {
std::streampos file_offset;
uint32_t resource_size;
std::string resource_name;
};

struct BIG4Header {
char id[4]; // must contain 'BIG4'
uint32_t x1;
uint32_t resource_count;
uint32_t x2;
};

std::vector<Resource> resc_list;
std::string filename;

public:
BIGFile(const std::string& filename);
// From what I've seen so far, you probably don't need a destructor.
uint32_t GetCount() { return resc_list.size(); }
std::string GetNames();
void Extract(const std::string&, const std::string&);
};

// Read into existing T. Return whether successful (unless the stream
// is configured to throw an exception on failure).
template <typename T>
bool readPrimitive(istream& is, T& result) {
is.read(reinterpret_cast<char *>(&result), sizeof(T));
return is;
}

// Read a new T. Return default T on failure (unless the stream
// is configured to throw an exception on failure).
template <typename T>
T readPrimitive(istream& is) {
T result;
readPrimitive(is, result);
return result;
}

BIGFile::BIGFile(const std::string& filename) : filename(filename) {
ifstream in;
// Bail out immediately if anything goes wrong, automatically cleaning things
// up due to RAII
in.exceptions(ios::bad_bit | ios::fail_bit | ios::eof_bit);
in.open(file.c_str());

BIG4Header b4h = readPrimitive<BIG4Header>(in);
char* resource_count_swapper = reinterpret_cast<char *>(&b4h.resource_count);
std::swap(resource_count_swapper[0], resource_count_swapper[3]);
std::swap(resource_count_swapper[1], resource_count_swapper[2]);

// construct a string from the header ID, explicitly specifying the range of
// bytes to use, so as to avoid problems with the lack of a null terminator.
std::string type(b4h.id, b4h.id + 4);
if (type != "BIG4") {
throw ios::failure("File type not recognized: " + type);
}

Resource r;
for (uint32_t i=0; i<resource_count; ++i) {
readPrimitive(in, r.file_offset);
readPrimitive(in, r.resource_size);
std::stringbuf sbuff;
in.get(sbuff,'\0');
r.resource_name = sbuff.str();
resc_list.push_back(r);
}
}

Share this post


Link to post
Share on other sites

This topic is 3809 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this