Blingu
Blingu

Reputation: 51

sprintf buffer issue, wrong assignment to char array

I got an issue with sprintf buffer. As you can see in the code down below I'm saving with sprintf a char array to the buffer, so pFile can check if there's a file named like that in the folder. If it's found, the buffer value will be assigned to timecycles[numCycles], and numCycles will be increased. Example: timecycles[0] = "timecyc1.dat". It works well, and as you can see in the console output it recognizes that there are only timecyc1.dat and timecyc5.dat in the folder. But as long as I want to read timecycles with a for loop, both indexes have the value "timecyc9.dat", eventhough it should be "timecyc1.dat" for timecycles[0] and "timecyc5.dat" for timecycles1. Second thing is, how can I write the code so readTimecycles() returns char* timecycles, and I could just initialize it in the main function with char* timecycles[9] = readTimecycles() or anything like that?

Console output

#include <iostream>
#include <cstdio>

char* timecycles[9];

void readTimecycles()
{
char buffer[256];
int numCycles = 0;
FILE* pFile = NULL;

for (int i = 1; i < 10; i++)
{
    sprintf(buffer, "timecyc%d.dat", i);
    pFile = fopen(buffer, "r");

    if (pFile != NULL)
    {
        timecycles[numCycles] = buffer;
        numCycles++;
        std::cout << buffer << std::endl; //to see if the buffer is correct
    }

}
   for (int i = 0; i < numCycles; i++)
   {
    std::cout << timecycles[i] << std::endl; //here's the issue with timecyc9.dat
   }
}

int main()
{
readTimecycles();

return 0;
}

Upvotes: 0

Views: 222

Answers (2)

john
john

Reputation: 88092

The fundamental problem is that your notion of a string doesn't match what a 'char array' is in C++. In particular you think that because you assign timecycles[numCycles] = buffer; somehow the chars of the char array are copied. But in C++ all that is being copied is a pointer, so timecycles ends up with multiple pointers to the same buffer. And that's not to mention the problem you will have that when you exit the readTimecycles function. At that point you will have multiple pointers to a buffer which no longer exists as it gets destroyed when you exit the readTimecycles function.

The way to fix this is to use C++ code that does match your expectations. In particular a std::string will copy in the way you expect it to. Here's how you can change your code to use std::string

#include <string>

std::string timecycles[9];

timecycles[numCycles] = buffer; // now this really does copy a string

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409482

With the assignment

timecycles[numCycles] = buffer;

you make all pointers point to the same buffer, since you only have a single buffer.

Since you're programming in C++ you could easily solve your problem by using std::string instead.


If I would remake your code into something a little-more C++-ish and less C-ish, it could look something like

std::array<std::string, 9> readTimeCycles()
{
    std::array<std::string, 9> timecycles;

    for (size_t i = 0; i < timecycles.size(); ++i)
    {
        // Format the file-name
        std::string filename = "timecyc" + std::to_string(i + 1) + ".dat";

        std::ifstream file(filename);
        if (file)
        {
            // File was opened okay
            timecycles[i] = filename;
        }
    }

    return timecycles;
}

References:

Upvotes: 1

Related Questions