C++ Segmentation Fault when loading Image Pixels from Binary File

Question

Doing some C++ for fun and have a problem where when I load an Image after doing some modifications to the image, it gives me segmentation fault. I feel like I'm missing something but I don't know where.

EDIT Here's the code for both the save and load function, (assume that all necessary header files are included):

    int Image::save(const char* filename)
    {
      if(filename == NULL)
      {
        return 1;
      }
      ///*
      ofstream outFile(filename, ios::out | ios::binary);
      if (!outFile)
      {
        return 1;
      }
      outFile.write(reinterpret_cast<char*>(&cols), sizeof(unsigned int));
      outFile.write(reinterpret_cast<char*>(&rows), sizeof(unsigned int));
      outFile.write(reinterpret_cast<char*>(pixels), sizeof(uint8_t) * cols * rows);
      outFile.close();
      return 0;
    }

    int Image::load(const char* filename)
    {
      if(filename == NULL)
      {
        return 1;
      }
      ///*
      ifstream inFile(filename, ios::in | ios::binary);
      if (!inFile)
      {
        return 1;
      }
      **//feels like the segmentation fault is happening here**

      inFile.read(reinterpret_cast<char*>(&cols), sizeof(unsigned int));
      inFile.read(reinterpret_cast<char*>(&rows), sizeof(unsigned int));
      inFile.read(reinterpret_cast<char*>(pixels), sizeof(uint8_t) * cols * rows);
      inFile.close();
      return 0;
    }

EDIT Here's the header file that I am working with:

class Image {

public:
  unsigned int cols;
  unsigned int rows;
  uint8_t* pixels;

...

/* Saves the image in the file filename. In a format that can be
     loaded by load().  Returns 0 on success, else a non-zero error
     code. */
  int save( const char* filename );

  /* Load an image from the file filename, replacing the current
     image size and data. The file is in a format that was saved by
     save().  Returns 0 success, else a non-zero error code . */
  int load( const char* filename );
};

I think it's probably in your write functions within save. When you are using char* it may be allowed only read operations. That's my first guess. Can you test just the save method and see if you achieve a seg fault? — Easton Bornemeier, Aug 03 '17 at 19:18
Why don't you just check exactly where it happens with a debugger? Also, what are rows, cols and pixels by the way? And why are they apparently global? — KjMag, Aug 03 '17 at 19:59
[Why is `iostream::eof` inside a loop condition considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). — molbdnilo, Aug 03 '17 at 20:07
@EastonBornemeier, the save function is working properly, just not the load function — , Aug 03 '17 at 21:09

zzxyz · Accepted Answer · 2017-08-03T22:29:49.533

2

You're moving the file pointer to the end of the file before trying to read it when you open it with ios::ate. You want to read from the beginning of the file, so ios::ate should be removed.

Also you are reading in a loop, and not writing in a loop. Your while should be an if, or just removed.

Also read does not adjust your pointer (or shouldn't...see my next point), but merely reads data into where you are pointing to. So the NULL check (if pixels==NULL) is nonsensical.

Also, you shouldn't be using the address-of operator (&) for pixels. pixels is already a pointer and both your read and write of this variable should have the & removed, like so:

inFile.read(reinterpret_cast<char*>(pixels), sizeof(uint8_t) * cols * rows);

You may find this helpful: http://boredzo.org/pointers/

edit:

    inFile.read(reinterpret_cast<char*>(&cols), sizeof(unsigned int));
    inFile.read(reinterpret_cast<char*>(&rows), sizeof(unsigned int));
    resize(cols, rows, 0);
    inFile.read(reinterpret_cast<char*>(pixels), sizeof(uint8_t) * cols * rows);

Your resize() needs to make sure the pointer isn't NULL before trying to delete it, and you probably should make fill() a separate function.

But at least do

int Image::resize(unsigned int width,  unsigned int height, uint8_t fillcolor)
{
  if (pixels != NULL)
      delete[] pixels;
  ...

edited Aug 03 '17 at 22:29

answered Aug 03 '17 at 20:14

zzxyz

2,794
1
12
31

so for the read function, it should read something like this `inFile.read(reinterpret_cast(&cols), sizeof(unsigned int));` `inFile.read(reinterpret_cast(&rows), sizeof(unsigned int));` `inFile.read(reinterpret_cast(pixels), sizeof(uint8_t) * cols * rows);` – Aug 03 '17 at 20:46
and for save `inFile.read(reinterpret_cast(pixels), sizeof(uint8_t) * cols * rows);` yet it's giving me free():invalid size – Aug 03 '17 at 21:15
Probably need to see main(). You need to be allocating memory for your pixels pointer. Ideally, the correct amount, like so: "pixels = new uint8_t[cols * rows];" – zzxyz Aug 03 '17 at 21:42
for this: `if (pixels != NULL) delete [] pixels;` where should this be put? – Aug 03 '17 at 22:28
I clarified in my answer – zzxyz Aug 03 '17 at 22:32

score 0 · Answer 2 · answered Aug 03 '17 at 20:28

In addition to @zzxyz's answer, you may be running into a problem with byte order. When you read cols and rows, c++ might order the bytes in integers from least significant to most significant (little endian), while the file could order the bytes from most significant to least significant (big endian), for example (see more here). This could give you values of cols and rows wildly different from what you expect, and reading cols * rows bytes could make inFile try to read far beyond the length of the file. I recommend checking or printing the values of cols and rows and making sure they line up with what you expect; if not, you'll have to reverse the order of the bytes in the integers.

C++ Segmentation Fault when loading Image Pixels from Binary File

2 Answers2