From 4013938da7e8460072e4c99d067580e168cdd72c Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:11:42 -0400 Subject: [PATCH 01/18] Ignored Eclipse files. --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index b8d83a3ec0..63a16a57a3 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ branches/ build/ dist/ testsuite/runtest.pyc + +.cproject +.project From f9aca8ce158a96837b462cac3d7df5740bbc8664 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:13:50 -0400 Subject: [PATCH 02/18] Sysutil::put_in_background -> returns true if fork was successful. --- src/libutil/sysutil.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/sysutil.cpp b/src/libutil/sysutil.cpp index b2278cf610..5ca816d78d 100644 --- a/src/libutil/sysutil.cpp +++ b/src/libutil/sysutil.cpp @@ -241,8 +241,8 @@ Sysutil::put_in_background (int, char* []) #if defined(__linux__) || defined(__GLIBC__) // Simplest case: - daemon (1, 1); - return true; + // daemon returns 0 if successful, thus return true if successful + return daemon (1, 1) == 0; #endif #ifdef __APPLE__ From 595b984ac2c2acbd28e7b8fe302867a67b0624ba Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:24:30 -0400 Subject: [PATCH 03/18] Fixes error where strncpy was copying max 200 chars which is greater than the structs' 100 char size. Lowered the max copy to 100 chars as is appropriate with the File Header. Removes two unused variables through comments. These have been tagged with FIXME. I do not fully comprehend the algorithm, but they are _not_ used. --- src/dpx.imageio/libdpx/DPXHeader.h | 2 +- src/libtexture/imagecache.cpp | 4 ++-- src/libtexture/texturesys.cpp | 19 ++++++++++--------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/dpx.imageio/libdpx/DPXHeader.h b/src/dpx.imageio/libdpx/DPXHeader.h index 7c9816918f..7f974ba9c8 100644 --- a/src/dpx.imageio/libdpx/DPXHeader.h +++ b/src/dpx.imageio/libdpx/DPXHeader.h @@ -1611,7 +1611,7 @@ namespace dpx inline void GenericHeader::SetCreator(const char *creat) { - ::strncpy(this->creator, creat, 200); + ::strncpy(this->creator, creat, 100); } inline void GenericHeader::Project(char *prj) const diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index 64bcc21d17..3c85c0f06e 100644 --- a/src/libtexture/imagecache.cpp +++ b/src/libtexture/imagecache.cpp @@ -980,7 +980,7 @@ ImageCacheImpl::find_file (ustring filename, // What if we've opened another file, with a different name, // but the SAME pixels? It can happen! Bad user, bad! But // let's save them from their own foolishness. - bool was_duplicate = false; +// bool was_duplicate = false; // FIXME Unused Variable, should this be checked in the loop? if (tf->fingerprint() && m_deduplicate) { // std::cerr << filename << " hash=" << tf->fingerprint() << "\n"; ImageCacheFile *dup = find_fingerprint (tf->fingerprint(), tf); @@ -999,7 +999,7 @@ ImageCacheImpl::find_file (ustring filename, tf->m_sample_border == dup->m_sample_border) { tf->duplicate (dup); tf->close (); - was_duplicate = true; +// was_duplicate = true; // FIXME Unused Variable // std::cerr << " duplicates " // << fingerfound->second.get()->filename() << "\n"; } diff --git a/src/libtexture/texturesys.cpp b/src/libtexture/texturesys.cpp index 4abc1d141d..68c9d2d678 100644 --- a/src/libtexture/texturesys.cpp +++ b/src/libtexture/texturesys.cpp @@ -1041,7 +1041,8 @@ TextureSystemImpl::texture_lookup (TextureFile &texturefile, float levelblend = 0; // The ellipse is made up of two axes which correspond to the x and y pixel // directions. Pick the longest one and take several samples along it. - float smajor, tmajor, sminor, tminor; + float smajor, tmajor; + //float sminor, tminor; // FIXME UNUSED_AXIS These are currently unused Axis float majorlength, minorlength; #if 0 @@ -1056,15 +1057,15 @@ TextureSystemImpl::texture_lookup (TextureFile &texturefile, minorlength = yfilt; smajor = dsdx; tmajor = dtdx; - sminor = dsdy; - tminor = dtdy; +// sminor = dsdy; // FIXME See UNUSED_AXIS +// tminor = dtdy; // FIXME See UNUSED_AXIS } else { majorlength = yfilt; minorlength = xfilt; smajor = dsdy; tmajor = dtdy; - sminor = dsdx; - tminor = dtdx; +// sminor = dsdx; // FIXME See UNUSED_AXIS +// tminor = dtdx; // FIXME See UNUSED_AXIS } #else // Do a bit more math and get the exact ellipse axis lengths, and @@ -1077,13 +1078,13 @@ TextureSystemImpl::texture_lookup (TextureFile &texturefile, if (axis) { smajor = dsdy; tmajor = dtdy; - sminor = dsdx; - tminor = dtdx; +// sminor = dsdx; // FIXME See UNUSED_AXIS +// tminor = dtdx; // FIXME See UNUSED_AXIS } else { smajor = dsdx; tmajor = dtdx; - sminor = dsdy; - tminor = dtdy; +// sminor = dsdy; // FIXME See UNUSED_AXIS +// tminor = dtdy; // FIXME See UNUSED_AXIS } #endif From f2a71faad1dd0f71883b3c389fcc1bdcb30be229 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:33:24 -0400 Subject: [PATCH 04/18] Fixes an unused variable. It was set to 0 then left there. I also formatted part of the method, it was hard to read. --- src/hdr.imageio/rgbe.cpp | 205 ++++++++++++++++++++------------------- 1 file changed, 104 insertions(+), 101 deletions(-) diff --git a/src/hdr.imageio/rgbe.cpp b/src/hdr.imageio/rgbe.cpp index 783b6a1275..b00566bde9 100644 --- a/src/hdr.imageio/rgbe.cpp +++ b/src/hdr.imageio/rgbe.cpp @@ -83,6 +83,7 @@ static int rgbe_error(int rgbe_error_code, const char *msg, char *errbuf) sprintf(errbuf,"RGBE error: %s\n",msg); else fprintf(stderr,"RGBE error: %s\n",msg); + break; } return RGBE_RETURN_FAILURE; } @@ -164,108 +165,110 @@ int RGBE_WriteHeader(FILE *fp, int width, int height, rgbe_header_info *info, int RGBE_ReadHeader(FILE *fp, int *width, int *height, rgbe_header_info *info, char *errbuf) { - char buf[128]; - int found_format; - float tempf; - int i; - - found_format = 0; - if (info) { - info->valid = 0; - info->programtype[0] = 0; - info->gamma = info->exposure = 1.0; - } - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == NULL) - return rgbe_error(rgbe_read_error,NULL, errbuf); - if ((buf[0] != '#')||(buf[1] != '?')) { - /* if you want to require the magic token then uncomment the next line */ - /*return rgbe_error(rgbe_format_error,"bad initial token"); */ - } - else if (info) { - info->valid |= RGBE_VALID_PROGRAMTYPE; - for(i=0;i<(int)sizeof(info->programtype)-1;i++) { - if ((buf[i+2] == 0) || isspace(buf[i+2])) - break; - info->programtype[i] = buf[i+2]; - } - info->programtype[i] = 0; - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) - return rgbe_error(rgbe_read_error,NULL, errbuf); - } - bool found_FORMAT_line = false; - for(;;) { - if ((buf[0] == 0)||(buf[0] == '\n')) { - if (found_FORMAT_line) - break; - return rgbe_error(rgbe_format_error,"no FORMAT specifier found", errbuf); - } - else if (strcmp(buf,"FORMAT=32-bit_rle_rgbe\n") == 0) { - found_FORMAT_line = true; - /* LG says no: break; // format found so break out of loop */ - } - else if (info && (sscanf(buf,"GAMMA=%g",&tempf) == 1)) { - info->gamma = tempf; - info->valid |= RGBE_VALID_GAMMA; - } - else if (info && (sscanf(buf,"EXPOSURE=%g",&tempf) == 1)) { - info->exposure = tempf; - info->valid |= RGBE_VALID_EXPOSURE; - } - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) - return rgbe_error(rgbe_read_error,NULL, errbuf); - } - if (strcmp(buf,"\n") != 0) { - printf ("Found '%s'\n", buf); - return rgbe_error(rgbe_format_error, - "missing blank line after FORMAT specifier", errbuf); - } - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) - return rgbe_error(rgbe_read_error,NULL, errbuf); + char buf[128]; + float tempf; + size_t i; - if (sscanf(buf,"-Y %d +X %d",height,width) == 2) { - if (info) { - info->orientation = 1; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"-Y %d -X %d",height,width) == 2) { - if (info) { - info->orientation = 2; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+Y %d -X %d",height,width) == 2) { - if (info) { - info->orientation = 3; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+Y %d +X %d",height,width) == 2) { - if (info) { - info->orientation = 4; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+X %d -Y %d",height,width) == 2) { - if (info) { - info->orientation = 5; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+X %d +Y %d",height,width) == 2) { - if (info) { - info->orientation = 6; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"-X %d +Y %d",height,width) == 2) { - if (info) { - info->orientation = 7; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"-X %d -Y %d",height,width) == 2) { - if (info) { - info->orientation = 8; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else { - return rgbe_error(rgbe_format_error,"missing image size specifier", errbuf); - } - return RGBE_RETURN_SUCCESS; + if (info) { + info->valid = 0; + info->programtype[0] = 0; + info->gamma = info->exposure = 1.0; + } + + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == NULL) + return rgbe_error(rgbe_read_error,NULL, errbuf); + + if ((buf[0] != '#')||(buf[1] != '?')) { + /* if you want to require the magic token then uncomment the next line */ + /*return rgbe_error(rgbe_format_error,"bad initial token"); */ + } else if (info) { + info->valid |= RGBE_VALID_PROGRAMTYPE; + for (i = 0; i < (int)sizeof(info->programtype)-1; i++) { + if ((buf[i + 2] == 0) || isspace(buf[i + 2])) + break; + info->programtype[i] = buf[i + 2]; + } + info->programtype[i] = 0; + if (fgets(buf, sizeof(buf)/sizeof(buf[0]), fp) == 0) + return rgbe_error(rgbe_read_error, NULL, errbuf); + } + + bool found_FORMAT_line = false; + for ( ; ; ) { + if ((buf[0] == 0)||(buf[0] == '\n')) { + if (found_FORMAT_line) + break; + return rgbe_error(rgbe_format_error,"no FORMAT specifier found", errbuf); + } + else if (strcmp(buf,"FORMAT=32-bit_rle_rgbe\n") == 0) { + found_FORMAT_line = true; + /* LG says no: break; // format found so break out of loop */ + } + else if (info && (sscanf(buf,"GAMMA=%g",&tempf) == 1)) { + info->gamma = tempf; + info->valid |= RGBE_VALID_GAMMA; + } + else if (info && (sscanf(buf,"EXPOSURE=%g",&tempf) == 1)) { + info->exposure = tempf; + info->valid |= RGBE_VALID_EXPOSURE; + } + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) + return rgbe_error(rgbe_read_error,NULL, errbuf); + } /* end for ( ; ; ) */ + + if (strcmp(buf,"\n") != 0) { + printf ("Found '%s'\n", buf); + return rgbe_error(rgbe_format_error, + "missing blank line after FORMAT specifier", errbuf); + } + + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) + return rgbe_error(rgbe_read_error,NULL, errbuf); + + if (sscanf(buf,"-Y %d +X %d", height, width) == 2) { + if (info) { + info->orientation = 1; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"-Y %d -X %d", height, width) == 2) { + if (info) { + info->orientation = 2; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+Y %d -X %d", height, width) == 2) { + if (info) { + info->orientation = 3; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+Y %d +X %d", height, width) == 2) { + if (info) { + info->orientation = 4; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+X %d -Y %d", height, width) == 2) { + if (info) { + info->orientation = 5; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+X %d +Y %d", height, width) == 2) { + if (info) { + info->orientation = 6; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"-X %d +Y %d", height, width) == 2) { + if (info) { + info->orientation = 7; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"-X %d -Y %d", height, width) == 2) { + if (info) { + info->orientation = 8; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else { + return rgbe_error(rgbe_format_error, "missing image size specifier", errbuf); + } + return RGBE_RETURN_SUCCESS; } /* simple write routine that does not use run length encoding */ From 01fc9d3e8a5c19a1c71c9531b47d27d600cf5039 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:53:22 -0400 Subject: [PATCH 05/18] Fixed a small issue with defining INLINE, now will be recognized on more platforms (if not all) --- src/hdr.imageio/rgbe.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hdr.imageio/rgbe.cpp b/src/hdr.imageio/rgbe.cpp index b00566bde9..020e732380 100644 --- a/src/hdr.imageio/rgbe.cpp +++ b/src/hdr.imageio/rgbe.cpp @@ -32,7 +32,7 @@ will only accept "RADIANCE" as the programtype. */ -#ifdef _CPLUSPLUS +#if defined(_CPLUSPLUS) || defined(__cplusplus) /* define if your compiler understands inline commands */ #define INLINE inline #else From 6d8493de7e73e42d0b3205ef71caa9046188ceed Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:54:01 -0400 Subject: [PATCH 06/18] Fixes two fread problems, both return false and act as other parts do. --- src/tiff.imageio/tiffinput.cpp | 6 +++++- src/webp.imageio/webpinput.cpp | 8 +++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/tiff.imageio/tiffinput.cpp b/src/tiff.imageio/tiffinput.cpp index 3db510aa30..7ed2e310e2 100644 --- a/src/tiff.imageio/tiffinput.cpp +++ b/src/tiff.imageio/tiffinput.cpp @@ -292,8 +292,12 @@ TIFFInput::valid_file (const std::string &filename) const if (! file) return false; // needs to be able to open unsigned short magic[2] = { 0, 0 }; - fread (magic, sizeof(unsigned short), 2, file); + size_t numRead = fread (magic, sizeof(unsigned short), 2, file); fclose (file); + if ( numRead != 2 ) { // fread failed + return false; + } + if (magic[0] != TIFF_LITTLEENDIAN && magic[0] != TIFF_BIGENDIAN) return false; // not the right byte order if ((magic[0] == TIFF_LITTLEENDIAN) != littleendian()) diff --git a/src/webp.imageio/webpinput.cpp b/src/webp.imageio/webpinput.cpp index ff516b5581..82b0f24bf9 100644 --- a/src/webp.imageio/webpinput.cpp +++ b/src/webp.imageio/webpinput.cpp @@ -82,7 +82,13 @@ WebpInput::open (const std::string &name, ImageSpec &spec) std::vector encoded_image; encoded_image.resize(m_image_size, 0); - fread(&encoded_image[0], sizeof(uint8_t), encoded_image.size(), m_file); + int numRead = fread(&encoded_image[0], sizeof(uint8_t), encoded_image.size(), m_file); + if (numRead != encoded_image.size()) { + error ("Did not read correct amount of images. (expected %d, read %d)", + encoded_image.size(), numRead); + close (); + return false; + } int width = 0, height = 0; if(!WebPGetInfo(&encoded_image[0], encoded_image.size(), &width, &height)) From c79ddb241b3eb9fb78d7bfed3b111eedbbb91029 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 9 Jul 2012 22:55:56 -0400 Subject: [PATCH 07/18] Fixes two unused variables, similar: two booleans for printed that were ignored. Little bit of copy paste. --- src/iinfo/iinfo.cpp | 4 ++-- src/oiiotool/printinfo.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/iinfo/iinfo.cpp b/src/iinfo/iinfo.cpp index e30711be7d..f1b695b30e 100644 --- a/src/iinfo/iinfo.cpp +++ b/src/iinfo/iinfo.cpp @@ -464,7 +464,7 @@ print_info (const std::string &filename, size_t namefieldlength, ImageInput *input, ImageSpec &spec, bool verbose, bool sum, long long &totalsize) { - bool printed = false; +// bool printed = false; // FIXME Unused completely int padlen = std::max (0, (int)namefieldlength - (int)filename.length()); std::string padding (padlen, ' '); @@ -523,7 +523,7 @@ print_info (const std::string &filename, size_t namefieldlength, if (! verbose && num_of_subimages == 1 && any_mipmapping) printf (" (+mipmap)"); printf ("\n"); - printed = true; +// printed = true; } if (verbose && num_of_subimages != 1) { diff --git a/src/oiiotool/printinfo.cpp b/src/oiiotool/printinfo.cpp index 12b3d8f37c..ab75e1c38d 100644 --- a/src/oiiotool/printinfo.cpp +++ b/src/oiiotool/printinfo.cpp @@ -475,7 +475,7 @@ OiioTool::print_info (const std::string &filename, field_re.assign (opt.metamatch, boost::regex::extended | boost::regex_constants::icase); - bool printed = false; +// bool printed = false; // FIXME Unused Completely int padlen = std::max (0, (int)opt.namefieldlength - (int)filename.length()); std::string padding (padlen, ' '); @@ -534,7 +534,7 @@ OiioTool::print_info (const std::string &filename, if (! opt.verbose && num_of_subimages == 1 && any_mipmapping) printf (" (+mipmap)"); printf ("\n"); - printed = true; +// printed = true; } if (opt.verbose && num_of_subimages != 1) { From c624285295eb7f467a771b402a73c9ee8d6f44a7 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 10 Jul 2012 00:19:38 -0400 Subject: [PATCH 08/18] Fixes all of the warnings in the .log supplied by Richard (@hobbes1069).. hopefully. --- src/bmp.imageio/bmp_pvt.cpp | 27 +-- src/bmp.imageio/bmpoutput.cpp | 4 +- src/fits.imageio/fitsoutput.cpp | 12 +- src/ico.imageio/icooutput.cpp | 89 ++++++++-- src/jpeg2000.imageio/jpeg2000output.cpp | 6 +- src/ptex.imageio/ptex/PtexWriter.cpp | 10 +- src/sgi.imageio/sgioutput.cpp | 38 ++-- src/targa.imageio/targaoutput.cpp | 224 ++++++++++++++++++------ src/webp.imageio/webpinput.cpp | 2 +- src/webp.imageio/webpoutput.cpp | 6 +- src/zfile.imageio/zfile.cpp | 19 +- 11 files changed, 321 insertions(+), 116 deletions(-) diff --git a/src/bmp.imageio/bmp_pvt.cpp b/src/bmp.imageio/bmp_pvt.cpp index fa72b6a19b..7e942c1d63 100644 --- a/src/bmp.imageio/bmp_pvt.cpp +++ b/src/bmp.imageio/bmp_pvt.cpp @@ -154,18 +154,21 @@ DibInformationHeader::write_header (FILE *fd) { if (bigendian ()) swap_endian (); - fwrite (&size, 1, sizeof(size), fd); - fwrite (&width, 1, sizeof(width), fd); - fwrite (&height, 1, sizeof(height), fd); - fwrite (&cplanes, 1, sizeof(cplanes), fd); - fwrite (&bpp, 1, sizeof(bpp), fd); - fwrite (&compression, 1, sizeof(compression), fd); - fwrite (&isize, 1, sizeof(isize), fd); - fwrite (&hres, 1, sizeof(hres), fd); - fwrite (&vres, 1, sizeof(vres), fd); - fwrite (&cpalete, 1, sizeof(cpalete), fd); - fwrite (&important, 1, sizeof(important), fd); - return true; + + size_t bytes = 0; + bytes += fwrite (&size, 1, sizeof(size), fd); + bytes += fwrite (&width, 1, sizeof(width), fd); + bytes += fwrite (&height, 1, sizeof(height), fd); + bytes += fwrite (&cplanes, 1, sizeof(cplanes), fd); + bytes += fwrite (&bpp, 1, sizeof(bpp), fd); + bytes += fwrite (&compression, 1, sizeof(compression), fd); + bytes += fwrite (&isize, 1, sizeof(isize), fd); + bytes += fwrite (&hres, 1, sizeof(hres), fd); + bytes += fwrite (&vres, 1, sizeof(vres), fd); + bytes += fwrite (&cpalete, 1, sizeof(cpalete), fd); + bytes += fwrite (&important, 1, sizeof(important), fd); + + return (bytes == (size_t)size); // bytes == size --> wrote correct amount } diff --git a/src/bmp.imageio/bmpoutput.cpp b/src/bmp.imageio/bmpoutput.cpp index 64c1c82053..d4bc5a7d46 100644 --- a/src/bmp.imageio/bmpoutput.cpp +++ b/src/bmp.imageio/bmpoutput.cpp @@ -111,8 +111,8 @@ BmpOutput::write_scanline (int y, int z, TypeDesc format, const void *data, for (int i = 0, iend = buf.size() - 2; i < iend; i += m_spec.nchannels) std::swap (buf[i], buf[i+2]); - fwrite (&buf[0], 1, buf.size (), m_fd); - return true; + size_t byte_count = fwrite (&buf[0], 1, buf.size (), m_fd); + return byte_count == buf.size (); // true if wrote all bytes (no error) } diff --git a/src/fits.imageio/fitsoutput.cpp b/src/fits.imageio/fitsoutput.cpp index a9b0eabc8e..37ec6afc25 100644 --- a/src/fits.imageio/fitsoutput.cpp +++ b/src/fits.imageio/fitsoutput.cpp @@ -118,10 +118,12 @@ FitsOutput::write_scanline (int y, int z, TypeDesc format, const void *data, data_tmp.size () / sizeof (double)); } - fwrite (&data_tmp[0], 1, data_tmp.size (), m_fd); + size_t byte_count = fwrite (&data_tmp[0], 1, data_tmp.size (), m_fd); fsetpos (m_fd, &m_filepos); - return true; + + //byte_count == data.size --> all written + return byte_count == data_tmp.size(); } @@ -204,8 +206,12 @@ FitsOutput::create_fits_header (void) const int hsize = HEADER_SIZE - header.size () % HEADER_SIZE; if (hsize) header.resize (header.size () + hsize, ' '); - fwrite (&header[0], 1, header.size (), m_fd); + size_t byte_count = fwrite (&header[0], 1, header.size (), m_fd); + if (byte_count != header.size ()) { + // FIXME Bad Write + error ("Bad header write (err %d)", byte_count); + } } diff --git a/src/ico.imageio/icooutput.cpp b/src/ico.imageio/icooutput.cpp index 123260052c..990751f295 100644 --- a/src/ico.imageio/icooutput.cpp +++ b/src/ico.imageio/icooutput.cpp @@ -219,7 +219,11 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&ico.type); swap_endian (&ico.count); } - fwrite (&ico, 1, sizeof(ico), m_file); + size_t byte_count = fwrite (&ico, 1, sizeof(ico), m_file); + if (byte_count != sizeof(ico)) { + error ("Bad file write in ico. (err: %d)", byte_count); + return false; + } m_offset = sizeof(ico_header) + sizeof(ico_subimage); } else { // we'll be appending data, so see what's already in the file @@ -245,7 +249,13 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, int len = ftell (m_file); unsigned char buf[512]; // append null data at the end of file so that we don't seek beyond eof - fwrite (buf, sizeof (ico_subimage), 1, m_file); + { + size_t byte_count = fwrite (buf, sizeof (ico_subimage), 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico open. (err: %d)", byte_count); + return false; + } + } // do the actual moving, 0.5kB per iteration int amount, skip = sizeof (ico_header) + sizeof (ico_subimage) @@ -259,7 +269,11 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, return false; fseek (m_file, skip + left - amount + sizeof (ico_subimage), SEEK_SET); - fwrite (buf, amount, 1, m_file); + size_t byte_count = fwrite (buf, amount, 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico. (err: %d)", byte_count); + return false; + } } // update header @@ -269,7 +283,14 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&ico.type); swap_endian (&ico.count); } - fwrite (&ico, sizeof (ico), 1, m_file); + + { + size_t byte_count = fwrite (&ico, sizeof (ico), 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico. (err: %d)", byte_count); + return false; + } + } // and finally, update the offsets in subimage headers to point to // their data correctly @@ -285,7 +306,12 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&temp); // roll back 4 bytes, we need to rewrite the value we just read fseek (m_file, -4, SEEK_CUR); - fwrite (&temp, sizeof (temp), 1, m_file); + size_t byte_count = fwrite (&temp, sizeof (temp), 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico. (err: %d)", byte_count); + return false; + } + // skip to the next subimage; subtract 4 bytes because that's how // much we've just written fseek (m_file, sizeof (ico_subimage) - 4, SEEK_CUR); @@ -318,7 +344,12 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&subimg.len); swap_endian (&subimg.ofs); } - fwrite (&subimg, 1, sizeof(subimg), m_file); + size_t byte_count = fwrite (&subimg, 1, sizeof(subimg), m_file); + if (byte_count != sizeof(subimg)) { + error ("Bad file write in ico open. (err: %d)", byte_count); + return false; + } + fseek (m_file, m_offset, SEEK_SET); if (m_want_png) { @@ -345,13 +376,23 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&bmi.height); swap_endian (&bmi.len); } - fwrite (&bmi, sizeof (bmi), 1, m_file); + + size_t byte_count = fwrite (&bmi, sizeof (bmi), 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico open. (err: %d)", byte_count); + return false; + } // append null data so that we don't seek beyond eof in write_scanline char buf[512]; memset (buf, 0, sizeof (buf)); - for (int left = bmi.len; left > 0; left -= sizeof (buf)) - fwrite (buf, std::min (left, (int)sizeof (buf)), 1, m_file); + for (int left = bmi.len; left > 0; left -= sizeof (buf)) { + size_t byte_count = fwrite (buf, std::min (left, (int)sizeof (buf)), 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico open. (err: %d)", byte_count); + return false; + } + } fseek (m_file, m_offset + sizeof (bmi), SEEK_SET); } @@ -416,31 +457,38 @@ ICOOutput::write_scanline (int y, int z, TypeDesc format, fseek (m_file, m_offset + sizeof (ico_bitmapinfo) + (m_spec.height - y - 1) * m_xor_slb, SEEK_SET); // write the XOR mask + size_t buff_size = 0; for (int x = 0; x < m_spec.width; x++) { switch (m_color_type) { // reuse PNG constants case PNG_COLOR_TYPE_GRAY: buf[0] = buf[1] = buf[2] = bdata[x]; - fwrite (buf, 3, 1, m_file); - break; + buff_size = 3; + break; case PNG_COLOR_TYPE_GRAY_ALPHA: buf[0] = buf[1] = buf[2] = bdata[x * 2 + 0]; buf[3] = bdata[x * 2 + 1]; - fwrite (buf, 4, 1, m_file); - break; + buff_size = 4; + break; case PNG_COLOR_TYPE_RGB: buf[0] = bdata[x * 3 + 2]; buf[1] = bdata[x * 3 + 1]; buf[2] = bdata[x * 3 + 0]; - fwrite (buf, 3, 1, m_file); - break; + buff_size = 3; + break; case PNG_COLOR_TYPE_RGB_ALPHA: buf[0] = bdata[x * 4 + 2]; buf[1] = bdata[x * 4 + 1]; buf[2] = bdata[x * 4 + 0]; buf[3] = bdata[x * 4 + 3]; - fwrite (buf, 4, 1, m_file); - break; + buff_size = 4; + break; + } + + size_t byte_count = fwrite (buf, buff_size, 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico write_scanline (err: %d)", byte_count); + return false; } } @@ -470,7 +518,12 @@ ICOOutput::write_scanline (int y, int z, TypeDesc format, break; } } - fwrite (&buf[0], 1, 1, m_file); + + size_t byte_count = fwrite (&buf[0], 1, 1, m_file); + if (byte_count != 1) { + error ("Bad file write in ico write_scanline. (err: %d)", byte_count); + return false; + } } } } diff --git a/src/jpeg2000.imageio/jpeg2000output.cpp b/src/jpeg2000.imageio/jpeg2000output.cpp index 52ca33a0da..412b54e25e 100644 --- a/src/jpeg2000.imageio/jpeg2000output.cpp +++ b/src/jpeg2000.imageio/jpeg2000output.cpp @@ -178,7 +178,11 @@ Jpeg2000Output::save_image() opj_encode(compressor, cio, m_image, NULL); - fwrite(cio->buffer, 1, cio_tell(cio), m_file); + size_t wb = fwrite(cio->buffer, 1, cio_tell(cio), m_file); + if (wb != (size_t)cio_tell(cio)) { + error ("Failed write jpeg2000::save_image (err: %d)", wb); + return false; + } opj_destroy_compress(compressor); opj_cio_close(cio); diff --git a/src/ptex.imageio/ptex/PtexWriter.cpp b/src/ptex.imageio/ptex/PtexWriter.cpp index 4745e32a12..d7670131e4 100644 --- a/src/ptex.imageio/ptex/PtexWriter.cpp +++ b/src/ptex.imageio/ptex/PtexWriter.cpp @@ -1395,8 +1395,12 @@ void PtexIncrWriter::finish() // rewrite extheader for updated editdatasize if (_extheader.editdatapos) { - _extheader.editdatasize = uint64_t(ftello(_fp)) - _extheader.editdatapos; - fseeko(_fp, HeaderSize, SEEK_SET); - fwrite(&_extheader, PtexUtils::min(uint32_t(ExtHeaderSize), _header.extheadersize), 1, _fp); + _extheader.editdatasize = uint64_t(ftello(_fp)) - _extheader.editdatapos; + fseeko(_fp, HeaderSize, SEEK_SET); + size_t byte_count = fwrite(&_extheader, PtexUtils::min(uint32_t(ExtHeaderSize), _header.extheadersize), 1, _fp); + if (byte_count != 1) { + // FIXME Bad Write + //error ("Bad write PtexIncrWriter::finish (err %d)", byte_count); + } } } diff --git a/src/sgi.imageio/sgioutput.cpp b/src/sgi.imageio/sgioutput.cpp index 554b6da051..2bbf893d3d 100644 --- a/src/sgi.imageio/sgioutput.cpp +++ b/src/sgi.imageio/sgioutput.cpp @@ -112,7 +112,12 @@ SgiOutput::write_scanline (int y, int z, TypeDesc format, const void *data, long scanline_offset = sgi_pvt::SGI_HEADER_LEN + (c * m_spec.height + y) * m_spec.width * bpc; fseek (m_fd, scanline_offset, SEEK_SET); - fwrite (&channeldata[0], 1, m_spec.width*bpc, m_fd); + size_t byte_count = fwrite (&channeldata[0], 1, m_spec.width * bpc, m_fd); + if (byte_count != (size_t)(m_spec.width * bpc)) { + // FIXME Bad Write + error ("Bad write sgi::write_scanline (err %d)", byte_count); + return false; + } } return true; @@ -174,20 +179,25 @@ SgiOutput::create_and_write_header() swap_endian(&sgi_header.colormap); } - fwrite(&sgi_header.magic, sizeof(sgi_header.magic), 1, m_fd); - fwrite(&sgi_header.storage, sizeof(sgi_header.storage), 1, m_fd); - fwrite(&sgi_header.bpc, sizeof(sgi_header.bpc), 1, m_fd); - fwrite(&sgi_header.dimension, sizeof(sgi_header.dimension), 1, m_fd); - fwrite(&sgi_header.xsize, sizeof(sgi_header.xsize), 1, m_fd); - fwrite(&sgi_header.ysize, sizeof(sgi_header.ysize), 1, m_fd); - fwrite(&sgi_header.zsize, sizeof(sgi_header.zsize), 1, m_fd); - fwrite(&sgi_header.pixmin, sizeof(sgi_header.pixmin), 1, m_fd); - fwrite(&sgi_header.pixmax, sizeof(sgi_header.pixmax), 1, m_fd); - fwrite(&sgi_header.dummy, sizeof(sgi_header.dummy), 1, m_fd); - fwrite(sgi_header.imagename, sizeof(sgi_header.imagename), 1, m_fd); - fwrite(&sgi_header.colormap, sizeof(sgi_header.colormap), 1, m_fd); + size_t byte_count = 0; + byte_count += fwrite(&sgi_header.magic, sizeof(sgi_header.magic), 1, m_fd); + byte_count += fwrite(&sgi_header.storage, sizeof(sgi_header.storage), 1, m_fd); + byte_count += fwrite(&sgi_header.bpc, sizeof(sgi_header.bpc), 1, m_fd); + byte_count += fwrite(&sgi_header.dimension, sizeof(sgi_header.dimension), 1, m_fd); + byte_count += fwrite(&sgi_header.xsize, sizeof(sgi_header.xsize), 1, m_fd); + byte_count += fwrite(&sgi_header.ysize, sizeof(sgi_header.ysize), 1, m_fd); + byte_count += fwrite(&sgi_header.zsize, sizeof(sgi_header.zsize), 1, m_fd); + byte_count += fwrite(&sgi_header.pixmin, sizeof(sgi_header.pixmin), 1, m_fd); + byte_count += fwrite(&sgi_header.pixmax, sizeof(sgi_header.pixmax), 1, m_fd); + byte_count += fwrite(&sgi_header.dummy, sizeof(sgi_header.dummy), 1, m_fd); + byte_count += fwrite(sgi_header.imagename, sizeof(sgi_header.imagename), 1, m_fd); + byte_count += fwrite(&sgi_header.colormap, sizeof(sgi_header.colormap), 1, m_fd); char dummy[404] = {0}; - fwrite(dummy, 404, 1, m_fd); + byte_count += fwrite(dummy, 404, 1, m_fd); + + if (byte_count != 13) { // FIXME The parameters in fwrite should be swapped to make error checking nicer + error ("Write failed in sgi::create_and_write_header (err: unknwn)"); + } } OIIO_PLUGIN_NAMESPACE_END diff --git a/src/targa.imageio/targaoutput.cpp b/src/targa.imageio/targaoutput.cpp index b0b95251d1..02445db48f 100644 --- a/src/targa.imageio/targaoutput.cpp +++ b/src/targa.imageio/targaoutput.cpp @@ -205,7 +205,8 @@ TGAOutput::open (const std::string &name, const ImageSpec &userspec, // due to struct packing, we may get a corrupt header if we just dump the // struct to the file; to adress that, write every member individually // save some typing -#define WH(memb) fwrite (&tga.memb, sizeof (tga.memb), 1, m_file) + size_t byte_count = 0; +#define WH(memb) byte_count += fwrite (&tga.memb, sizeof (tga.memb), 1, m_file) WH(idlen); WH(cmap_type); WH(type); @@ -219,10 +220,20 @@ TGAOutput::open (const std::string &name, const ImageSpec &userspec, WH(bpp); WH(attr); #undef WH + if (byte_count != 12) { // number of members + error ("Failed write targa::open (err: unknwn)"); + return false; + } + // dump comment to file, don't bother about null termination - if (tga.idlen) - fwrite (id.c_str(), tga.idlen, 1, m_file); + if (tga.idlen) { + byte_count = fwrite (id.c_str(), tga.idlen, 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::open (err: %d)", byte_count); + return false; + } + } return true; } @@ -232,8 +243,19 @@ TGAOutput::open (const std::string &name, const ImageSpec &userspec, bool TGAOutput::close () { - if (m_file) { - // write out the TGA 2.0 data fields + // This call is made a lot: +#define WRITE_TMP_INT(count) { \ + size_t byte_count = fwrite (&tmpint, count, 1, m_file); \ + if (byte_count != 1) { \ + error ("Failed write targa::close (err: %d)", byte_count); \ + return false; \ + } \ + } + + if (m_file) { + size_t byte_count = 0; + + // write out the TGA 2.0 data fields // FIXME: write out the developer area; according to Larry, // it's probably safe to ignore it altogether until someone complains @@ -259,10 +281,16 @@ TGAOutput::close () if (bigendian()) swap_endian (&ofs_thumb); // dump thumbnail size - fwrite (&tw, 1, 1, m_file); - fwrite (&th, 1, 1, m_file); + size_t byte_count = 0; + byte_count += fwrite (&tw, 1, 1, m_file); + byte_count += fwrite (&th, 1, 1, m_file); // dump thumbnail data - fwrite (p->data(), p->datasize(), 1, m_file); + byte_count += fwrite (p->data(), p->datasize(), 1, m_file); + if (byte_count != 3) { + error ("Failed write targa::close (err: %d)", byte_count); + + return false; + } } } } @@ -281,17 +309,25 @@ TGAOutput::close () short tmpint = 495; if (bigendian()) swap_endian (&tmpint); - fwrite (&tmpint, sizeof (tmpint), 1, m_file); + WRITE_TMP_INT(1); tmpint = 0; // author std::string tmpstr = m_spec.get_string_attribute ("Artist", ""); - fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), + + byte_count = fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + + return false; + } + // fill the rest with zeros - for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) - fwrite (&tmpint, 1, 1, m_file); + for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) { + WRITE_TMP_INT(1); + } // image comment tmpstr = m_spec.get_string_attribute ("ImageDescription", ""); @@ -303,21 +339,29 @@ TGAOutput::close () // on line breaks, fill the remainder of the line with zeros if (p[pos] == '\n') { while ((w + 1) % 81 != 0) { - fwrite (&tmpint, 1, 1, m_file); + WRITE_TMP_INT(1); + w++; } continue; } - fwrite (&p[pos], 1, 1, m_file); + + byte_count = fwrite (&p[pos], 1, 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + + return false; + } // null-terminate each line if ((w + 1) % 81 == 0) { - fwrite (&tmpint, 1, 1, m_file); + WRITE_TMP_INT(1); w++; } } // fill the rest with zeros - for (; w < 324; w++) - fwrite (&tmpint, 1, 1, m_file); + for (; w < 324; w++) { + WRITE_TMP_INT(1); + } } // timestamp @@ -337,22 +381,39 @@ TGAOutput::close () swap_endian (&i); swap_endian (&s); } - fwrite (&m, sizeof (m), 1, m_file); - fwrite (&d, sizeof (d), 1, m_file); - fwrite (&y, sizeof (y), 1, m_file); - fwrite (&h, sizeof (h), 1, m_file); - fwrite (&i, sizeof (i), 1, m_file); - fwrite (&s, sizeof (s), 1, m_file); + byte_count = fwrite (&m, sizeof (m), 1, m_file); + byte_count += fwrite (&d, sizeof (d), 1, m_file); + byte_count += fwrite (&y, sizeof (y), 1, m_file); + byte_count += fwrite (&h, sizeof (h), 1, m_file); + byte_count += fwrite (&i, sizeof (i), 1, m_file); + byte_count += fwrite (&s, sizeof (s), 1, m_file); + if (byte_count != 6) { + error ("Failed write targa::close (err: %d)", byte_count); + return false; + } } // job ID tmpstr = m_spec.get_string_attribute ("DocumentName", ""); - fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), + byte_count = fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + + return false; + } + // fill the rest with zeros - for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) + for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) { fwrite (&tmpint, 1, 1, m_file); + byte_count = fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), + 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + return false; + } + } // job time tmpstr = m_spec.get_string_attribute ("targa:JobTime", ""); { @@ -366,18 +427,27 @@ TGAOutput::close () swap_endian (&m); swap_endian (&s); } - fwrite (&h, sizeof (h), 1, m_file); - fwrite (&m, sizeof (m), 1, m_file); - fwrite (&s, sizeof (s), 1, m_file); + byte_count = fwrite (&h, sizeof (h), 1, m_file); + byte_count += fwrite (&m, sizeof (m), 1, m_file); + byte_count += fwrite (&s, sizeof (s), 1, m_file); + if (byte_count != 3) { + error ("Failed write targa::close (err: unknwn)"); + return false; + } } // software ID - we advertise ourselves tmpstr = OIIO_INTRO_STRING; - fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), + + byte_count += fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + return false; + } // fill the rest with zeros for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) - fwrite (&tmpint, 1, 1, m_file); + WRITE_TMP_INT(1); // software version { @@ -386,14 +456,18 @@ TGAOutput::close () + OIIO_VERSION_PATCH; if (bigendian()) swap_endian (&v); - fwrite (&v, sizeof (v), 1, m_file); - fwrite (&tmpint, 1, 1, m_file); + byte_count += fwrite (&v, sizeof (v), 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + return false; + } + WRITE_TMP_INT(1) } // key colour // FIXME: what do we save here? - fwrite (&tmpint, 2, 1, m_file); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); // pixel aspect ratio { @@ -403,16 +477,16 @@ TGAOutput::close () // FIXME: invent a smarter way to convert to a vulgar fraction? // numerator tmpint = (unsigned short)(ratio * 10000.f); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); // denominator tmpint = 10000; - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); // reset tmpint value tmpint = 0; } else { // just dump two zeros in there - fwrite (&tmpint, 2, 1, m_file); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); } } @@ -426,16 +500,16 @@ TGAOutput::close () // is needed, thus the expansion by 10 // numerator tmpint = (unsigned short)(gamma * 10.f); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); // denominator tmpint = 10; - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); // reset tmpint value tmpint = 0; } else { // just dump two zeros in there - fwrite (&tmpint, 2, 1, m_file); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); } } @@ -443,28 +517,40 @@ TGAOutput::close () // FIXME: support this once it becomes clear how it's actually supposed // to be used... the spec is very unclear about this // for the time being just dump four NULL bytes - fwrite (&tmpint, 2, 1, m_file); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); // offset to thumbnail (endiannes has already been accounted for) - fwrite (&ofs_thumb, 4, 1, m_file); + byte_count = fwrite (&ofs_thumb, 4, 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + return false; + } // offset to scanline table // not used very widely, don't bother unless someone complains - fwrite (&tmpint, 2, 1, m_file); - fwrite (&tmpint, 2, 1, m_file); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); // alpha type { unsigned char at = (m_spec.nchannels % 2 == 0) ? TGA_ALPHA_USEFUL : TGA_ALPHA_NONE; - fwrite (&at, 1, 1, m_file); + byte_count = fwrite (&at, 1, 1, m_file); + if (byte_count != 1) { + error ("Failed write targa::close (err: %d)", byte_count); + return false; + } } // write out the TGA footer - fwrite (&foot.ofs_ext, 1, sizeof (foot.ofs_ext), m_file); - fwrite (&foot.ofs_dev, 1, sizeof (foot.ofs_dev), m_file); - fwrite (&foot.signature, 1, sizeof (foot.signature), m_file); + byte_count = fwrite (&foot.ofs_ext, 1, sizeof (foot.ofs_ext), m_file); + byte_count += fwrite (&foot.ofs_dev, 1, sizeof (foot.ofs_dev), m_file); + byte_count += fwrite (&foot.signature, 1, sizeof (foot.signature), m_file); + if (byte_count != sizeof(tga_footer)) { + error ("Failed write targa::close (err: unknwn)", byte_count); + return false; + } // close the stream fclose (m_file); @@ -474,6 +560,8 @@ TGAOutput::close () init (); // re-initialize return true; // How can we fail? // Epicly. -- IneQuation + +#undef WRITE_TMP_INT } @@ -486,9 +574,13 @@ TGAOutput::flush_rlp (unsigned char *buf, int size) return; // write packet header unsigned char h = (size - 1) | 0x80; - fwrite (&h, 1, 1, m_file); + size_t byte_count = fwrite (&h, 1, 1, m_file); // write packet pixel - fwrite (buf, m_spec.nchannels, 1, m_file); + byte_count += fwrite (buf, m_spec.nchannels, 1, m_file); + if (byte_count != 2) { + error ("Failed write targa::flush_rlp (err: unknwn)", byte_count); + return; + } } @@ -508,7 +600,11 @@ TGAOutput::flush_rawp (unsigned char *& src, int size, int start) for (int i = 0; i < size; i++) { if (n <= 2) { // 1- and 2-channels can write directly - fwrite (src+start, 1, n, m_file); + size_t b = fwrite (src+start, 1, n, m_file); + if (b != (size_t)n) { + error ("Write fail targa::flush_rawp (err: %d)", b); + return; + } } else { // 3- and 4-channel must swap red and blue buf[0] = src[(start + i) * n + 2]; @@ -516,7 +612,12 @@ TGAOutput::flush_rawp (unsigned char *& src, int size, int start) buf[2] = src[(start + i) * n + 0]; if (n > 3) buf[3] = src[(start + i) * n + 3]; - fwrite (buf, 1, n, m_file); + + size_t b = fwrite (buf, 1, n, m_file); + if (b != (size_t)n) { + error ("Write fail targa::flush_rawp (err: %d)", b); + return; + } } } } @@ -660,14 +761,23 @@ TGAOutput::write_scanline (int y, int z, TypeDesc format, fseek(m_file, 18 + m_idlen + (m_spec.height - y - 1) * w * n, SEEK_SET); if (n <= 2) { // 1- and 2-channels can write directly - fwrite (bdata, n, w, m_file); + size_t c = fwrite (bdata, n, w, m_file); + if (c != (size_t)w) { + error ("Failed write targa::write_scanline (err: %d)", c); + return false; + } } else { // 3- and 4-channels must swap R and B std::vector buf; buf.assign (bdata, bdata + n*w); for (int x = 0; x < m_spec.width; x++) std::swap (buf[x*n], buf[x*n+2]); - fwrite (&buf[0], n, w, m_file); + + size_t c = fwrite (&buf[0], n, w, m_file); + if (c != (size_t)w) { + error ("Failed write targa::write_scanline (err: %d)", c); + return false; + } } } diff --git a/src/webp.imageio/webpinput.cpp b/src/webp.imageio/webpinput.cpp index 82b0f24bf9..98b3b499ce 100644 --- a/src/webp.imageio/webpinput.cpp +++ b/src/webp.imageio/webpinput.cpp @@ -82,7 +82,7 @@ WebpInput::open (const std::string &name, ImageSpec &spec) std::vector encoded_image; encoded_image.resize(m_image_size, 0); - int numRead = fread(&encoded_image[0], sizeof(uint8_t), encoded_image.size(), m_file); + size_t numRead = fread(&encoded_image[0], sizeof(uint8_t), encoded_image.size(), m_file); if (numRead != encoded_image.size()) { error ("Did not read correct amount of images. (expected %d, read %d)", encoded_image.size(), numRead); diff --git a/src/webp.imageio/webpoutput.cpp b/src/webp.imageio/webpoutput.cpp index c08e1a486a..7f995e7c63 100644 --- a/src/webp.imageio/webpoutput.cpp +++ b/src/webp.imageio/webpoutput.cpp @@ -70,7 +70,11 @@ static int WebpImageWriter(const uint8_t* img_data, size_t data_size, const WebPPicture* const webp_img) { FILE *out_file = (FILE*)webp_img->custom_ptr; - fwrite (img_data, data_size, sizeof(uint8_t), out_file); + size_t wb = fwrite (img_data, data_size, sizeof(uint8_t), out_file); + if (wb != sizeof(uint8_t)) { + //FIXME Bad write occurred + } + return 1; } diff --git a/src/zfile.imageio/zfile.cpp b/src/zfile.imageio/zfile.cpp index 343ce88b74..bd4a4e5b4a 100644 --- a/src/zfile.imageio/zfile.cpp +++ b/src/zfile.imageio/zfile.cpp @@ -316,8 +316,13 @@ ZfileOutput::open (const std::string &name, const ImageSpec &userspec, if (m_gz) gzwrite (m_gz, &header, sizeof(header)); - else - fwrite (&header, sizeof(header), 1, m_file); + else { + size_t b = fwrite (&header, sizeof(header), 1, m_file); + if (b != 1) { + error ("Failed write zfile::open (err: %d)", b); + return false; + } + } return true; } @@ -358,8 +363,14 @@ ZfileOutput::write_scanline (int y, int z, TypeDesc format, if (m_gz) gzwrite (m_gz, data, m_spec.width*sizeof(float)); - else - fwrite (data, sizeof(float), m_spec.width, m_file); + else { + size_t b = fwrite (data, sizeof(float), m_spec.width, m_file); + if (b != (size_t)m_spec.width) { + error ("Failed write zfile::open (err: %d)", b); + return false; + } + } + return true; } From 212178af99aa4e46bcf44721cc1aa90f1450a675 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Wed, 11 Jul 2012 21:27:54 -0400 Subject: [PATCH 09/18] Fixes several ambiguous ::strncpy calls. --- src/dpx.imageio/libdpx/DPXHeader.h | 52 +++++++++++++++--------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/dpx.imageio/libdpx/DPXHeader.h b/src/dpx.imageio/libdpx/DPXHeader.h index 7f974ba9c8..3eefd54663 100644 --- a/src/dpx.imageio/libdpx/DPXHeader.h +++ b/src/dpx.imageio/libdpx/DPXHeader.h @@ -1532,13 +1532,13 @@ namespace dpx inline void GenericHeader::Version(char *v) const { - ::strncpy(v, this->version, 8); + ::strncpy(v, this->version, sizeof(this->version)); v[8] = '\0'; } inline void GenericHeader::SetVersion(const char * v) { - ::strncpy(this->version, v, 8); + ::strncpy(this->version, v, sizeof(this->version)); } inline U32 GenericHeader::FileSize() const @@ -1583,57 +1583,57 @@ namespace dpx inline void GenericHeader::FileName(char *fn) const { - ::strncpy(fn, this->fileName, 100); + ::strncpy(fn, this->fileName, sizeof(this->fileName)); fn[100] = '\0'; } inline void GenericHeader::SetFileName(const char *fn) { - ::strncpy(this->fileName, fn, 100); + ::strncpy(this->fileName, fn, sizeof(this->fileName)); } inline void GenericHeader::CreationTimeDate(char *ct) const { - ::strncpy(ct, this->creationTimeDate, 24); + ::strncpy(ct, this->creationTimeDate, sizeof(this->creationTimeDate)); ct[24] = '\0'; } inline void GenericHeader::SetCreationTimeDate(const char *ct) { - ::strncpy(this->creationTimeDate, ct, 24); + ::strncpy(this->creationTimeDate, ct, sizeof(this->creationTimeDate)); } inline void GenericHeader::Creator(char *creat) const { - ::strncpy(creat, this->creator, 200); + ::strncpy(creat, this->creator, sizeof(this->creator)); creat[200] = '\0'; } inline void GenericHeader::SetCreator(const char *creat) { - ::strncpy(this->creator, creat, 100); + ::strncpy(this->creator, creat, sizeof(this->creator)); } inline void GenericHeader::Project(char *prj) const { - ::strncpy(prj, this->project, 200); + ::strncpy(prj, this->project, sizeof(this->project)); prj[200] = '\0'; } inline void GenericHeader::SetProject(const char *prj) { - ::strncpy(this->project, prj, 200); + ::strncpy(this->project, prj, sizeof(this->project)); } inline void GenericHeader::Copyright(char *copy) const { - ::strncpy(copy, this->copyright, 200); + ::strncpy(copy, this->copyright, sizeof(this->copyright)); copy[200] = '\0'; } inline void GenericHeader::SetCopyright(const char *copy) { - ::strncpy(this->copyright, copy, 200); + ::strncpy(this->copyright, copy, sizeof(this->copyright)); } inline U32 GenericHeader::EncryptKey() const @@ -1971,46 +1971,46 @@ namespace dpx inline void GenericHeader::SourceImageFileName(char *fn) const { - ::strncpy(fn, this->sourceImageFileName, 100); + ::strncpy(fn, this->sourceImageFileName, sizeof(this->sourceImageFileName)); fn[100] = '\0'; } inline void GenericHeader::SetSourceImageFileName(const char *fn) { - ::strncpy(this->sourceImageFileName, fn, 100); + ::strncpy(this->sourceImageFileName, fn, sizeof(this->sourceImageFileName)); } inline void GenericHeader::SourceTimeDate(char *td) const { - ::strncpy(td, this->sourceTimeDate, 24); + ::strncpy(td, this->sourceTimeDate, sizeof(this->sourceTimeDate)); td[24] = '\0'; } inline void GenericHeader::SetSourceTimeDate(const char *td) { - ::strncpy(this->sourceTimeDate, td, 24); + ::strncpy(this->sourceTimeDate, td, sizeof(this->sourceTimeDate)); } inline void GenericHeader::InputDevice(char *dev) const { - ::strncpy(dev, this->inputDevice, 32); + ::strncpy(dev, this->inputDevice, sizeof(this->inputDevice)); dev[32] = '\0'; } inline void GenericHeader::SetInputDevice(const char *dev) { - ::strncpy(this->inputDevice, dev, 32); + ::strncpy(this->inputDevice, dev, sizeof(this->inputDevice)); } inline void GenericHeader::InputDeviceSerialNumber(char *sn) const { - ::strncpy(sn, this->inputDeviceSerialNumber, 32); + ::strncpy(sn, this->inputDeviceSerialNumber, sizeof(this->inputDeviceSerialNumber)); sn[32] = '\0'; } inline void GenericHeader::SetInputDeviceSerialNumber(const char *sn) { - ::strncpy(this->inputDeviceSerialNumber, sn, 32); + ::strncpy(this->inputDeviceSerialNumber, sn, sizeof(this->inputDeviceSerialNumber)); } inline U16 GenericHeader::Border(const int i) const @@ -2066,13 +2066,13 @@ namespace dpx inline void IndustryHeader::Format(char *fmt) const { - ::strncpy(fmt, this->format, 32); + ::strncpy(fmt, this->format, sizeof(this->format)); fmt[32] = '\0'; } inline void IndustryHeader::SetFormat(const char *fmt) { - ::strncpy(this->format, fmt, 32); + ::strncpy(this->format, fmt, sizeof(this->format)); } inline U32 IndustryHeader::FramePosition() const @@ -2127,24 +2127,24 @@ namespace dpx inline void IndustryHeader::FrameId(char *id) const { - ::strncpy(id, this->frameId, 32); + ::strncpy(id, this->frameId, sizeof(this->frameId)); id[32] = '\0'; } inline void IndustryHeader::SetFrameId(const char *id) { - ::strncpy(this->frameId, id, 32); + ::strncpy(this->frameId, id, sizeof(this->frameId)); } inline void IndustryHeader::SlateInfo(char *slate) const { - ::strncpy(slate, this->slateInfo, 100); + ::strncpy(slate, this->slateInfo, sizeof(this->slateInfo)); slate[100] = '\0'; } inline void IndustryHeader::SetSlateInfo(const char *slate) { - ::strncpy(this->slateInfo, slate, 100); + ::strncpy(this->slateInfo, slate, sizeof(this->slateInfo)); } From a471c7375a89e937d9c4ad5723a3a25031bab419 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Thu, 19 Jul 2012 10:34:40 -0400 Subject: [PATCH 10/18] Uses the proposed solution by Larry. If this passes, I can continue on and do the rest. --- src/bmp.imageio/bmp_pvt.cpp | 154 +++++++++++++++++++++--------------- 1 file changed, 92 insertions(+), 62 deletions(-) diff --git a/src/bmp.imageio/bmp_pvt.cpp b/src/bmp.imageio/bmp_pvt.cpp index 7e942c1d63..dee6466584 100644 --- a/src/bmp.imageio/bmp_pvt.cpp +++ b/src/bmp.imageio/bmp_pvt.cpp @@ -33,21 +33,40 @@ OIIO_PLUGIN_NAMESPACE_BEGIN namespace bmp_pvt { +/// Helper - write, with error detection +template +bool fwrite (FILE *fd, const T &buf, size_t nitems=1) { + size_t itemsize = sizeof(T); + size_t n = std::fwrite (&buf, itemsize, nitems, fd); +// if (n != nitems) +// error ("Write error: wrote %d records of %d", (int)n, (int)nitems); + return n == nitems; +} +/// Helper - read, with error detection +template +bool fread (FILE *fd, T &buf, size_t nitems=1, size_t itemsize=sizeof(T)) { + // size_t itemsize = sizeof(T); + size_t n = std::fread (&buf, itemsize, nitems, fd); +// if (n != nitems) +// error ("Write error: wrote %d records of %d", (int)n, (int)nitems); + return n == nitems; +} bool BmpFileHeader::read_header (FILE *fd) { - int byte_count = 0; - byte_count += fread (&magic, 1, sizeof (magic), fd); - byte_count += fread (&fsize, 1, sizeof (fsize), fd); - byte_count += fread (&res1, 1, sizeof (res1), fd); - byte_count += fread (&res2, 1, sizeof (res2), fd); - byte_count += fread (&offset, 1, sizeof (offset), fd); + if (!fread(fd, magic) || + !fread(fd, fsize) || + !fread(fd, res1) || + !fread(fd, res2) || + !fread(fd, offset)) { + return false; + } if (bigendian ()) swap_endian (); - return (byte_count == BMP_HEADER_SIZE); + return true; } @@ -58,13 +77,15 @@ BmpFileHeader::write_header (FILE *fd) if (bigendian ()) swap_endian (); - int byte_count = 0; - byte_count += fwrite (&magic, 1, sizeof (magic), fd); - byte_count += fwrite (&fsize, 1, sizeof (fsize), fd); - byte_count += fwrite (&res1, 1, sizeof (res1), fd); - byte_count += fwrite (&res2, 1, sizeof (res2), fd); - byte_count += fwrite (&offset, 1, sizeof (offset), fd); - return (byte_count == BMP_HEADER_SIZE); + if (!fwrite(fd, magic) || + !fwrite(fd, fsize) || + !fwrite(fd, res1) || + !fwrite(fd, res2) || + !fwrite(fd, offset)) { + return false; + } + + return true; } @@ -98,53 +119,61 @@ BmpFileHeader::swap_endian (void) bool DibInformationHeader::read_header (FILE *fd) { - int byte_count = 0; - byte_count += fread (&size, 1, sizeof (size), fd); + if (!fread (fd, size)) + return false; if (size == WINDOWS_V3 || size == WINDOWS_V4) { - byte_count += fread (&width, 1, sizeof (width), fd); - byte_count += fread (&height, 1, sizeof (height), fd); - byte_count += fread (&cplanes, 1, sizeof (cplanes), fd); - byte_count += fread (&bpp, 1, sizeof (bpp), fd); - byte_count += fread (&compression, 1, sizeof (compression), fd); - byte_count += fread (&isize, 1, sizeof (isize), fd); - byte_count += fread (&hres, 1, sizeof (hres), fd); - byte_count += fread (&vres, 1, sizeof (vres), fd); - byte_count += fread (&cpalete, 1, sizeof (cpalete), fd); - byte_count += fread (&important, 1, sizeof (important), fd); + if (!fread(fd, width) || + !fread(fd, height) || + !fread(fd, cplanes) || + !fread(fd, bpp) || + !fread(fd, compression) || + !fread(fd, isize) || + !fread(fd, hres) || + !fread(fd, vres) || + !fread(fd, cpalete) || + !fread(fd, important)) { + return false; + } + if (size == WINDOWS_V4) { - byte_count += fread (&red_mask, 1, sizeof (red_mask), fd); - byte_count += fread (&blue_mask, 1, sizeof (blue_mask), fd); - byte_count += fread (&green_mask, 1, sizeof (green_mask), fd); - byte_count += fread (&cs_type, 1, sizeof (cs_type), fd); - byte_count += fread (&red_x, 1, sizeof (red_x), fd); - byte_count += fread (&red_y, 1, sizeof (red_y), fd); - byte_count += fread (&red_z, 1, sizeof (red_z), fd); - byte_count += fread (&green_x, 1, sizeof (green_x), fd); - byte_count += fread (&green_y, 1, sizeof (green_y), fd); - byte_count += fread (&green_z, 1, sizeof (green_z), fd); - byte_count += fread (&blue_x, 1, sizeof (blue_x), fd); - byte_count += fread (&blue_y, 1, sizeof (blue_y), fd); - byte_count += fread (&blue_z, 1, sizeof (blue_z), fd); - byte_count += fread (&gamma_x, 1, sizeof (gamma_x), fd); - byte_count += fread (&gamma_y, 1, sizeof (gamma_y), fd); - byte_count += fread (&gamma_z, 1, sizeof (gamma_z), fd); int32_t dummy; - byte_count += fread (&dummy, 1, sizeof (dummy), fd); + + if (!fread (fd, red_mask) || + !fread (fd, blue_mask) || + !fread (fd, green_mask) || + !fread (fd, cs_type) || + !fread (fd, red_x) || + !fread (fd, red_y) || + !fread (fd, red_z) || + !fread (fd, green_x) || + !fread (fd, green_y) || + !fread (fd, green_z) || + !fread (fd, blue_x) || + !fread (fd, blue_y) || + !fread (fd, blue_z) || + !fread (fd, gamma_x) || + !fread (fd, gamma_y) || + !fread (fd, gamma_z) || + !fread (fd, dummy)) { + return false; + } } } else if (size == OS2_V1) { // this fileds are smaller then in WINDOWS_Vx headers, // so we use hardcoded counts - byte_count += fread (&width, 1, 2, fd); - byte_count += fread (&height, 1, 2, fd); - byte_count += fread (&cplanes, 1, 2, fd); - byte_count += fread (&bpp, 1, 2, fd); + if (!fread (fd, width, 1, 2) || // + !fread (fd, height, 1, 2) || + !fread (fd, cplanes, 1, 2) || + !fread (fd, bpp, 1, 2)) { + return false; + } } if (bigendian ()) swap_endian (); - return (byte_count == size); + return true; } @@ -155,20 +184,21 @@ DibInformationHeader::write_header (FILE *fd) if (bigendian ()) swap_endian (); - size_t bytes = 0; - bytes += fwrite (&size, 1, sizeof(size), fd); - bytes += fwrite (&width, 1, sizeof(width), fd); - bytes += fwrite (&height, 1, sizeof(height), fd); - bytes += fwrite (&cplanes, 1, sizeof(cplanes), fd); - bytes += fwrite (&bpp, 1, sizeof(bpp), fd); - bytes += fwrite (&compression, 1, sizeof(compression), fd); - bytes += fwrite (&isize, 1, sizeof(isize), fd); - bytes += fwrite (&hres, 1, sizeof(hres), fd); - bytes += fwrite (&vres, 1, sizeof(vres), fd); - bytes += fwrite (&cpalete, 1, sizeof(cpalete), fd); - bytes += fwrite (&important, 1, sizeof(important), fd); - - return (bytes == (size_t)size); // bytes == size --> wrote correct amount + if (!fwrite(fd, size) || + !fwrite (fd, width) || + !fwrite (fd, height) || + !fwrite (fd, cplanes) || + !fwrite (fd, bpp) || + !fwrite (fd, compression) || + !fwrite (fd, isize) || + !fwrite (fd, hres) || + !fwrite (fd, vres) || + !fwrite (fd, cpalete) || + !fwrite (fd, important)) { + return false; + } + + return (true); } From b93138ad3a2273a4a92c337c5338e68346f31625 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 23 Jul 2012 10:15:57 -0400 Subject: [PATCH 11/18] Revert "Fixes an unused variable. It was set to 0 then left there. I also" This reverts commit f2a71faad1dd0f71883b3c389fcc1bdcb30be229. --- src/hdr.imageio/rgbe.cpp | 205 +++++++++++++++++++-------------------- 1 file changed, 101 insertions(+), 104 deletions(-) diff --git a/src/hdr.imageio/rgbe.cpp b/src/hdr.imageio/rgbe.cpp index 020e732380..9ddbc68fe4 100644 --- a/src/hdr.imageio/rgbe.cpp +++ b/src/hdr.imageio/rgbe.cpp @@ -83,7 +83,6 @@ static int rgbe_error(int rgbe_error_code, const char *msg, char *errbuf) sprintf(errbuf,"RGBE error: %s\n",msg); else fprintf(stderr,"RGBE error: %s\n",msg); - break; } return RGBE_RETURN_FAILURE; } @@ -165,110 +164,108 @@ int RGBE_WriteHeader(FILE *fp, int width, int height, rgbe_header_info *info, int RGBE_ReadHeader(FILE *fp, int *width, int *height, rgbe_header_info *info, char *errbuf) { - char buf[128]; - float tempf; - size_t i; - - if (info) { - info->valid = 0; - info->programtype[0] = 0; - info->gamma = info->exposure = 1.0; - } - - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == NULL) - return rgbe_error(rgbe_read_error,NULL, errbuf); - - if ((buf[0] != '#')||(buf[1] != '?')) { - /* if you want to require the magic token then uncomment the next line */ - /*return rgbe_error(rgbe_format_error,"bad initial token"); */ - } else if (info) { - info->valid |= RGBE_VALID_PROGRAMTYPE; - for (i = 0; i < (int)sizeof(info->programtype)-1; i++) { - if ((buf[i + 2] == 0) || isspace(buf[i + 2])) - break; - info->programtype[i] = buf[i + 2]; - } - info->programtype[i] = 0; - if (fgets(buf, sizeof(buf)/sizeof(buf[0]), fp) == 0) - return rgbe_error(rgbe_read_error, NULL, errbuf); - } - - bool found_FORMAT_line = false; - for ( ; ; ) { - if ((buf[0] == 0)||(buf[0] == '\n')) { - if (found_FORMAT_line) - break; - return rgbe_error(rgbe_format_error,"no FORMAT specifier found", errbuf); - } - else if (strcmp(buf,"FORMAT=32-bit_rle_rgbe\n") == 0) { - found_FORMAT_line = true; - /* LG says no: break; // format found so break out of loop */ - } - else if (info && (sscanf(buf,"GAMMA=%g",&tempf) == 1)) { - info->gamma = tempf; - info->valid |= RGBE_VALID_GAMMA; - } - else if (info && (sscanf(buf,"EXPOSURE=%g",&tempf) == 1)) { - info->exposure = tempf; - info->valid |= RGBE_VALID_EXPOSURE; - } - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) - return rgbe_error(rgbe_read_error,NULL, errbuf); - } /* end for ( ; ; ) */ - - if (strcmp(buf,"\n") != 0) { - printf ("Found '%s'\n", buf); - return rgbe_error(rgbe_format_error, - "missing blank line after FORMAT specifier", errbuf); - } - - if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) - return rgbe_error(rgbe_read_error,NULL, errbuf); + char buf[128]; + int found_format; + float tempf; + int i; + + found_format = 0; + if (info) { + info->valid = 0; + info->programtype[0] = 0; + info->gamma = info->exposure = 1.0; + } + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == NULL) + return rgbe_error(rgbe_read_error,NULL, errbuf); + if ((buf[0] != '#')||(buf[1] != '?')) { + /* if you want to require the magic token then uncomment the next line */ + /*return rgbe_error(rgbe_format_error,"bad initial token"); */ + } + else if (info) { + info->valid |= RGBE_VALID_PROGRAMTYPE; + for(i=0;i<(int)sizeof(info->programtype)-1;i++) { + if ((buf[i+2] == 0) || isspace(buf[i+2])) + break; + info->programtype[i] = buf[i+2]; + } + info->programtype[i] = 0; + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) + return rgbe_error(rgbe_read_error,NULL, errbuf); + } + bool found_FORMAT_line = false; + for(;;) { + if ((buf[0] == 0)||(buf[0] == '\n')) { + if (found_FORMAT_line) + break; + return rgbe_error(rgbe_format_error,"no FORMAT specifier found", errbuf); + } + else if (strcmp(buf,"FORMAT=32-bit_rle_rgbe\n") == 0) { + found_FORMAT_line = true; + /* LG says no: break; // format found so break out of loop */ + } + else if (info && (sscanf(buf,"GAMMA=%g",&tempf) == 1)) { + info->gamma = tempf; + info->valid |= RGBE_VALID_GAMMA; + } + else if (info && (sscanf(buf,"EXPOSURE=%g",&tempf) == 1)) { + info->exposure = tempf; + info->valid |= RGBE_VALID_EXPOSURE; + } + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) + return rgbe_error(rgbe_read_error,NULL, errbuf); + } + if (strcmp(buf,"\n") != 0) { + printf ("Found '%s'\n", buf); + return rgbe_error(rgbe_format_error, + "missing blank line after FORMAT specifier", errbuf); + } + if (fgets(buf,sizeof(buf)/sizeof(buf[0]),fp) == 0) + return rgbe_error(rgbe_read_error,NULL, errbuf); - if (sscanf(buf,"-Y %d +X %d", height, width) == 2) { - if (info) { - info->orientation = 1; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"-Y %d -X %d", height, width) == 2) { - if (info) { - info->orientation = 2; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+Y %d -X %d", height, width) == 2) { - if (info) { - info->orientation = 3; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+Y %d +X %d", height, width) == 2) { - if (info) { - info->orientation = 4; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+X %d -Y %d", height, width) == 2) { - if (info) { - info->orientation = 5; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"+X %d +Y %d", height, width) == 2) { - if (info) { - info->orientation = 6; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"-X %d +Y %d", height, width) == 2) { - if (info) { - info->orientation = 7; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else if (sscanf(buf,"-X %d -Y %d", height, width) == 2) { - if (info) { - info->orientation = 8; - info->valid |= RGBE_VALID_ORIENTATION; - } - } else { - return rgbe_error(rgbe_format_error, "missing image size specifier", errbuf); - } - return RGBE_RETURN_SUCCESS; + if (sscanf(buf,"-Y %d +X %d",height,width) == 2) { + if (info) { + info->orientation = 1; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"-Y %d -X %d",height,width) == 2) { + if (info) { + info->orientation = 2; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+Y %d -X %d",height,width) == 2) { + if (info) { + info->orientation = 3; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+Y %d +X %d",height,width) == 2) { + if (info) { + info->orientation = 4; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+X %d -Y %d",height,width) == 2) { + if (info) { + info->orientation = 5; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"+X %d +Y %d",height,width) == 2) { + if (info) { + info->orientation = 6; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"-X %d +Y %d",height,width) == 2) { + if (info) { + info->orientation = 7; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else if (sscanf(buf,"-X %d -Y %d",height,width) == 2) { + if (info) { + info->orientation = 8; + info->valid |= RGBE_VALID_ORIENTATION; + } + } else { + return rgbe_error(rgbe_format_error,"missing image size specifier", errbuf); + } + return RGBE_RETURN_SUCCESS; } /* simple write routine that does not use run length encoding */ From 01c4d1a3da70f21afa0f3e11901eb7404b512e7c Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 23 Jul 2012 10:18:44 -0400 Subject: [PATCH 12/18] Redoes the small changes in hdr/rgbe.cpp. This doesn't have any formatting done. --- src/hdr.imageio/rgbe.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/hdr.imageio/rgbe.cpp b/src/hdr.imageio/rgbe.cpp index 9ddbc68fe4..cfcc5853e7 100644 --- a/src/hdr.imageio/rgbe.cpp +++ b/src/hdr.imageio/rgbe.cpp @@ -83,6 +83,7 @@ static int rgbe_error(int rgbe_error_code, const char *msg, char *errbuf) sprintf(errbuf,"RGBE error: %s\n",msg); else fprintf(stderr,"RGBE error: %s\n",msg); + break; } return RGBE_RETURN_FAILURE; } @@ -165,11 +166,9 @@ int RGBE_ReadHeader(FILE *fp, int *width, int *height, rgbe_header_info *info, char *errbuf) { char buf[128]; - int found_format; float tempf; - int i; + size_t i; - found_format = 0; if (info) { info->valid = 0; info->programtype[0] = 0; From bde4ba84186f1655ff9a6cb714ece2c4f80fbc6a Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Mon, 23 Jul 2012 10:46:09 -0400 Subject: [PATCH 13/18] Fixes some fread/write calls in Ptex --- src/ptex.imageio/ptex/PtexWriter.cpp | 35 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/ptex.imageio/ptex/PtexWriter.cpp b/src/ptex.imageio/ptex/PtexWriter.cpp index d7670131e4..849a203055 100644 --- a/src/ptex.imageio/ptex/PtexWriter.cpp +++ b/src/ptex.imageio/ptex/PtexWriter.cpp @@ -504,8 +504,8 @@ int PtexWriterBase::writeBlock(FILE* fp, const void* data, int size) { if (!_ok) return 0; if (!fwrite(data, size, 1, fp)) { - setError("PtexWriter error: file write failed"); - return 0; + setError("PtexWriter error: file write failed"); + return 0; } return size; } @@ -545,8 +545,8 @@ int PtexWriterBase::writeZipBlock(FILE* fp, const void* data, int size, bool fin int PtexWriterBase::readBlock(FILE* fp, void* data, int size) { if (!fread(data, size, 1, fp)) { - setError("PtexWriter error: temp file read failed"); - return 0; + setError("PtexWriter error: temp file read failed"); + return 0; } return size; } @@ -1213,10 +1213,10 @@ PtexIncrWriter::PtexIncrWriter(const char* path, FILE* fp, // make sure existing header matches if (!fread(&_header, PtexIO::HeaderSize, 1, fp) || _header.magic != Magic) { - std::stringstream str; - str << "Not a ptex file: " << path; - setError(str.str()); - return; + std::stringstream str; + str << "Not a ptex file: " << path; + setError(str.str()); + return; } bool headerMatch = (mt == _header.meshtype && @@ -1225,20 +1225,20 @@ PtexIncrWriter::PtexIncrWriter(const char* path, FILE* fp, alphachan == int(_header.alphachan) && nfaces == int(_header.nfaces)); if (!headerMatch) { - std::stringstream str; - str << "PtexWriter::edit error: header doesn't match existing file, " - << "conversions not currently supported"; - setError(str.str()); + std::stringstream str; + str << "PtexWriter::edit error: header doesn't match existing file, " + << "conversions not currently supported"; + setError(str.str()); return; } // read extended header memset(&_extheader, 0, sizeof(_extheader)); if (!fread(&_extheader, PtexUtils::min(uint32_t(ExtHeaderSize), _header.extheadersize), 1, fp)) { - std::stringstream str; - str << "Error reading extended header: " << path; - setError(str.str()); - return; + std::stringstream str; + str << "Error reading extended header: " << path; + setError(str.str()); + return; } // seek to end of file to append @@ -1397,8 +1397,7 @@ void PtexIncrWriter::finish() if (_extheader.editdatapos) { _extheader.editdatasize = uint64_t(ftello(_fp)) - _extheader.editdatapos; fseeko(_fp, HeaderSize, SEEK_SET); - size_t byte_count = fwrite(&_extheader, PtexUtils::min(uint32_t(ExtHeaderSize), _header.extheadersize), 1, _fp); - if (byte_count != 1) { + if (!fwrite(&_extheader, PtexUtils::min(uint32_t(ExtHeaderSize), _header.extheadersize), 1, _fp)) { // FIXME Bad Write //error ("Bad write PtexIncrWriter::finish (err %d)", byte_count); } From 0f866488d695730e716b647908c4b15e37dc2330 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 24 Jul 2012 10:25:11 -0400 Subject: [PATCH 14/18] Fixes targa files. --- src/targa.imageio/targaoutput.cpp | 182 +++++++++++------------------- 1 file changed, 67 insertions(+), 115 deletions(-) diff --git a/src/targa.imageio/targaoutput.cpp b/src/targa.imageio/targaoutput.cpp index 02445db48f..11fef76cde 100644 --- a/src/targa.imageio/targaoutput.cpp +++ b/src/targa.imageio/targaoutput.cpp @@ -79,6 +79,15 @@ class TGAOutput : public ImageOutput { /// Helper function to flush a run-length packet /// inline void flush_rlp (unsigned char *buf, int size); + + /// Helper - write, with error detection + template + bool fwrite (const T &buf, size_t nitems=1, size_t itemsize=sizeof(T)) { + size_t n = std::fwrite (&buf, itemsize, nitems, m_file); + if (n != nitems) + error ("Write error: wrote %d records of %d", (int)n, (int)nitems); + return n == nitems; + } }; @@ -205,32 +214,25 @@ TGAOutput::open (const std::string &name, const ImageSpec &userspec, // due to struct packing, we may get a corrupt header if we just dump the // struct to the file; to adress that, write every member individually // save some typing - size_t byte_count = 0; -#define WH(memb) byte_count += fwrite (&tga.memb, sizeof (tga.memb), 1, m_file) - WH(idlen); - WH(cmap_type); - WH(type); - WH(cmap_first); - WH(cmap_length); - WH(cmap_size); - WH(x_origin); - WH(y_origin); - WH(width); - WH(height); - WH(bpp); - WH(attr); -#undef WH - if (byte_count != 12) { // number of members - error ("Failed write targa::open (err: unknwn)"); + if (!fwrite(tga.idlen) || + !fwrite(tga.cmap_type) || + !fwrite(tga.type) || + !fwrite(tga.cmap_first) || + !fwrite(tga.cmap_length) || + !fwrite(tga.cmap_size) || + !fwrite(tga.x_origin) || + !fwrite(tga.y_origin) || + !fwrite(tga.width) || + !fwrite(tga.height) || + !fwrite(tga.bpp) || + !fwrite(tga.attr)) { return false; } // dump comment to file, don't bother about null termination if (tga.idlen) { - byte_count = fwrite (id.c_str(), tga.idlen, 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::open (err: %d)", byte_count); + if (!fwrite(id.c_str(), tga.idlen)) { return false; } } @@ -245,17 +247,13 @@ TGAOutput::close () { // This call is made a lot: #define WRITE_TMP_INT(count) { \ - size_t byte_count = fwrite (&tmpint, count, 1, m_file); \ - if (byte_count != 1) { \ - error ("Failed write targa::close (err: %d)", byte_count); \ + if (!fwrite (tmpint, count)) { \ return false; \ } \ } if (m_file) { - size_t byte_count = 0; - - // write out the TGA 2.0 data fields + // write out the TGA 2.0 data fields // FIXME: write out the developer area; according to Larry, // it's probably safe to ignore it altogether until someone complains @@ -281,15 +279,10 @@ TGAOutput::close () if (bigendian()) swap_endian (&ofs_thumb); // dump thumbnail size - size_t byte_count = 0; - byte_count += fwrite (&tw, 1, 1, m_file); - byte_count += fwrite (&th, 1, 1, m_file); - // dump thumbnail data - byte_count += fwrite (p->data(), p->datasize(), 1, m_file); - if (byte_count != 3) { - error ("Failed write targa::close (err: %d)", byte_count); - - return false; + if (!fwrite (tw) || + !fwrite (th) || + !fwrite (p->data(), p->datasize())) { + return false; } } } @@ -315,12 +308,7 @@ TGAOutput::close () // author std::string tmpstr = m_spec.get_string_attribute ("Artist", ""); - - byte_count = fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), - 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - + if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { return false; } @@ -346,12 +334,9 @@ TGAOutput::close () continue; } - byte_count = fwrite (&p[pos], 1, 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - - return false; - } + if (!fwrite (p[pos])) { + return false; + } // null-terminate each line if ((w + 1) % 81 == 0) { WRITE_TMP_INT(1); @@ -381,36 +366,26 @@ TGAOutput::close () swap_endian (&i); swap_endian (&s); } - byte_count = fwrite (&m, sizeof (m), 1, m_file); - byte_count += fwrite (&d, sizeof (d), 1, m_file); - byte_count += fwrite (&y, sizeof (y), 1, m_file); - byte_count += fwrite (&h, sizeof (h), 1, m_file); - byte_count += fwrite (&i, sizeof (i), 1, m_file); - byte_count += fwrite (&s, sizeof (s), 1, m_file); - if (byte_count != 6) { - error ("Failed write targa::close (err: %d)", byte_count); + if (!fwrite(m) || + !fwrite(d) || + !fwrite(y) || + !fwrite(h) || + !fwrite(i) || + !fwrite(s)) { return false; } } // job ID tmpstr = m_spec.get_string_attribute ("DocumentName", ""); - byte_count = fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), - 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - + if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { return false; } // fill the rest with zeros for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) { - fwrite (&tmpint, 1, 1, m_file); - byte_count = fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), - 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - + if (!fwrite (tmpint) || + !fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { return false; } } @@ -427,11 +402,9 @@ TGAOutput::close () swap_endian (&m); swap_endian (&s); } - byte_count = fwrite (&h, sizeof (h), 1, m_file); - byte_count += fwrite (&m, sizeof (m), 1, m_file); - byte_count += fwrite (&s, sizeof (s), 1, m_file); - if (byte_count != 3) { - error ("Failed write targa::close (err: unknwn)"); + if (!fwrite(h) || + !fwrite(m) || + !fwrite(s)) { return false; } } @@ -439,10 +412,7 @@ TGAOutput::close () // software ID - we advertise ourselves tmpstr = OIIO_INTRO_STRING; - byte_count += fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)), - 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); + if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { return false; } // fill the rest with zeros @@ -456,11 +426,9 @@ TGAOutput::close () + OIIO_VERSION_PATCH; if (bigendian()) swap_endian (&v); - byte_count += fwrite (&v, sizeof (v), 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - return false; - } + if (!fwrite(v)) { + return false; + } WRITE_TMP_INT(1) } @@ -521,11 +489,9 @@ TGAOutput::close () WRITE_TMP_INT(2); // offset to thumbnail (endiannes has already been accounted for) - byte_count = fwrite (&ofs_thumb, 4, 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - return false; - } + if (!fwrite(&ofs_thumb)) { + return false; + } // offset to scanline table // not used very widely, don't bother unless someone complains @@ -536,19 +502,15 @@ TGAOutput::close () { unsigned char at = (m_spec.nchannels % 2 == 0) ? TGA_ALPHA_USEFUL : TGA_ALPHA_NONE; - byte_count = fwrite (&at, 1, 1, m_file); - if (byte_count != 1) { - error ("Failed write targa::close (err: %d)", byte_count); - return false; - } + if (!fwrite(at)) { + return false; + } } // write out the TGA footer - byte_count = fwrite (&foot.ofs_ext, 1, sizeof (foot.ofs_ext), m_file); - byte_count += fwrite (&foot.ofs_dev, 1, sizeof (foot.ofs_dev), m_file); - byte_count += fwrite (&foot.signature, 1, sizeof (foot.signature), m_file); - if (byte_count != sizeof(tga_footer)) { - error ("Failed write targa::close (err: unknwn)", byte_count); + if (!fwrite(&foot.ofs_ext) || + !fwrite(&foot.ofs_dev) || + !fwrite(&foot.signature)) { return false; } @@ -574,11 +536,9 @@ TGAOutput::flush_rlp (unsigned char *buf, int size) return; // write packet header unsigned char h = (size - 1) | 0x80; - size_t byte_count = fwrite (&h, 1, 1, m_file); // write packet pixel - byte_count += fwrite (buf, m_spec.nchannels, 1, m_file); - if (byte_count != 2) { - error ("Failed write targa::flush_rlp (err: unknwn)", byte_count); + if (!fwrite(h) || !fwrite (buf, m_spec.nchannels)) { + // do something intelligent? return; } } @@ -593,16 +553,15 @@ TGAOutput::flush_rawp (unsigned char *& src, int size, int start) return; // write packet header unsigned char h = (size - 1) & ~0x80; - fwrite (&h, 1, 1, m_file); + if (!fwrite (h)) + return; // rewind the scanline and flush packet pixels unsigned char buf[4]; int n = m_spec.nchannels; for (int i = 0; i < size; i++) { if (n <= 2) { // 1- and 2-channels can write directly - size_t b = fwrite (src+start, 1, n, m_file); - if (b != (size_t)n) { - error ("Write fail targa::flush_rawp (err: %d)", b); + if (!fwrite (src+start, n)) { return; } } else { @@ -612,12 +571,9 @@ TGAOutput::flush_rawp (unsigned char *& src, int size, int start) buf[2] = src[(start + i) * n + 0]; if (n > 3) buf[3] = src[(start + i) * n + 3]; - - size_t b = fwrite (buf, 1, n, m_file); - if (b != (size_t)n) { - error ("Write fail targa::flush_rawp (err: %d)", b); - return; - } + if (!fwrite (buf, n)) { + return; + } } } } @@ -761,9 +717,7 @@ TGAOutput::write_scanline (int y, int z, TypeDesc format, fseek(m_file, 18 + m_idlen + (m_spec.height - y - 1) * w * n, SEEK_SET); if (n <= 2) { // 1- and 2-channels can write directly - size_t c = fwrite (bdata, n, w, m_file); - if (c != (size_t)w) { - error ("Failed write targa::write_scanline (err: %d)", c); + if (!fwrite (bdata, n, w)) { return false; } } else { @@ -773,9 +727,7 @@ TGAOutput::write_scanline (int y, int z, TypeDesc format, for (int x = 0; x < m_spec.width; x++) std::swap (buf[x*n], buf[x*n+2]); - size_t c = fwrite (&buf[0], n, w, m_file); - if (c != (size_t)w) { - error ("Failed write targa::write_scanline (err: %d)", c); + if (!fwrite (&buf[0], n, w)) { return false; } } From 8d8edb19e91d8a28ec23fe99f1894b92cd1794ce Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 24 Jul 2012 10:25:58 -0400 Subject: [PATCH 15/18] fixes sgi --- src/sgi.imageio/sgi_pvt.h | 18 +++++++++++++++++ src/sgi.imageio/sgioutput.cpp | 38 +++++++++++++++-------------------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/sgi.imageio/sgi_pvt.h b/src/sgi.imageio/sgi_pvt.h index efaeacf910..6b80c2987f 100644 --- a/src/sgi.imageio/sgi_pvt.h +++ b/src/sgi.imageio/sgi_pvt.h @@ -157,6 +157,24 @@ class SgiOutput : public ImageOutput { } void create_and_write_header(); + + /// Helper - write, with error detection + template + bool fwrite (const T &buf, size_t nitems=1, size_t itemsize=sizeof(T)) { + size_t n = std::fwrite (&buf, itemsize, nitems, m_fd); + if (n != nitems) + error ("Write error: wrote %d records of %d", (int)n, (int)nitems); + return n == nitems; + } + + /// Helper - read, with error detection + template + bool fread (T &buf, size_t nitems=1, size_t itemsize=sizeof(T)) { + size_t n = std::fread (&buf, itemsize, nitems, m_fd); + if (n != nitems) + error ("Write error: wrote %d records of %d", (int)n, (int)nitems); + return n == nitems; + } }; diff --git a/src/sgi.imageio/sgioutput.cpp b/src/sgi.imageio/sgioutput.cpp index 2bbf893d3d..3769795a53 100644 --- a/src/sgi.imageio/sgioutput.cpp +++ b/src/sgi.imageio/sgioutput.cpp @@ -45,8 +45,6 @@ OIIO_PLUGIN_EXPORTS_BEGIN }; OIIO_PLUGIN_EXPORTS_END - - bool SgiOutput::open (const std::string &name, const ImageSpec &spec, OpenMode mode) @@ -112,10 +110,8 @@ SgiOutput::write_scanline (int y, int z, TypeDesc format, const void *data, long scanline_offset = sgi_pvt::SGI_HEADER_LEN + (c * m_spec.height + y) * m_spec.width * bpc; fseek (m_fd, scanline_offset, SEEK_SET); - size_t byte_count = fwrite (&channeldata[0], 1, m_spec.width * bpc, m_fd); - if (byte_count != (size_t)(m_spec.width * bpc)) { + if (!fwrite (channeldata[0], m_spec.width * bpc)) { // FIXME Bad Write - error ("Bad write sgi::write_scanline (err %d)", byte_count); return false; } } @@ -179,24 +175,22 @@ SgiOutput::create_and_write_header() swap_endian(&sgi_header.colormap); } - size_t byte_count = 0; - byte_count += fwrite(&sgi_header.magic, sizeof(sgi_header.magic), 1, m_fd); - byte_count += fwrite(&sgi_header.storage, sizeof(sgi_header.storage), 1, m_fd); - byte_count += fwrite(&sgi_header.bpc, sizeof(sgi_header.bpc), 1, m_fd); - byte_count += fwrite(&sgi_header.dimension, sizeof(sgi_header.dimension), 1, m_fd); - byte_count += fwrite(&sgi_header.xsize, sizeof(sgi_header.xsize), 1, m_fd); - byte_count += fwrite(&sgi_header.ysize, sizeof(sgi_header.ysize), 1, m_fd); - byte_count += fwrite(&sgi_header.zsize, sizeof(sgi_header.zsize), 1, m_fd); - byte_count += fwrite(&sgi_header.pixmin, sizeof(sgi_header.pixmin), 1, m_fd); - byte_count += fwrite(&sgi_header.pixmax, sizeof(sgi_header.pixmax), 1, m_fd); - byte_count += fwrite(&sgi_header.dummy, sizeof(sgi_header.dummy), 1, m_fd); - byte_count += fwrite(sgi_header.imagename, sizeof(sgi_header.imagename), 1, m_fd); - byte_count += fwrite(&sgi_header.colormap, sizeof(sgi_header.colormap), 1, m_fd); char dummy[404] = {0}; - byte_count += fwrite(dummy, 404, 1, m_fd); - - if (byte_count != 13) { // FIXME The parameters in fwrite should be swapped to make error checking nicer - error ("Write failed in sgi::create_and_write_header (err: unknwn)"); + if (!fwrite(sgi_header.magic) || + !fwrite(sgi_header.storage) || + !fwrite(sgi_header.bpc) || + !fwrite(sgi_header.dimension) || + !fwrite(sgi_header.xsize) || + !fwrite(sgi_header.ysize) || + !fwrite(sgi_header.zsize) || + !fwrite(sgi_header.pixmin) || + !fwrite(sgi_header.pixmax) || + !fwrite(sgi_header.dummy) || + !fwrite(sgi_header.imagename, 80, 1) || + !fwrite(sgi_header.colormap) || + + !fwrite(dummy, 404, 1)) { + // FIXME do something intelligent } } From a13ad66eae8058fd3326e1ab9a428dbbcc9b2acb Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 24 Jul 2012 10:35:11 -0400 Subject: [PATCH 16/18] Updates .ico --- src/ico.imageio/icooutput.cpp | 77 +++++++++++++++-------------------- 1 file changed, 32 insertions(+), 45 deletions(-) diff --git a/src/ico.imageio/icooutput.cpp b/src/ico.imageio/icooutput.cpp index 990751f295..c3a3b58dd3 100644 --- a/src/ico.imageio/icooutput.cpp +++ b/src/ico.imageio/icooutput.cpp @@ -91,7 +91,18 @@ class ICOOutput : public ImageOutput { /// Helper: read, with error detection /// - bool fread (void *buf, size_t itemsize, size_t nitems) { + template + bool fwrite (const T buf, size_t nitems = 1, size_t itemsize = sizeof(T)) { + size_t n = ::fwrite (&buf, itemsize, nitems, m_file); + if (n != nitems) + error ("Write error"); + return n == nitems; + } + + /// Helper: read, with error detection + /// + template + bool fread (T *buf, size_t nitems=1, size_t itemsize=sizeof(T)) { size_t n = ::fread (buf, itemsize, nitems, m_file); if (n != nitems) error ("Read error"); @@ -219,15 +230,13 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&ico.type); swap_endian (&ico.count); } - size_t byte_count = fwrite (&ico, 1, sizeof(ico), m_file); - if (byte_count != sizeof(ico)) { - error ("Bad file write in ico. (err: %d)", byte_count); - return false; + if (!fwrite(ico)) { + return false; } m_offset = sizeof(ico_header) + sizeof(ico_subimage); } else { // we'll be appending data, so see what's already in the file - if (! fread (&ico, 1, sizeof(ico))) + if (! fread (&ico)) return false; if (bigendian()) { // ICOs are little endian @@ -249,12 +258,8 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, int len = ftell (m_file); unsigned char buf[512]; // append null data at the end of file so that we don't seek beyond eof - { - size_t byte_count = fwrite (buf, sizeof (ico_subimage), 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico open. (err: %d)", byte_count); - return false; - } + if (!fwrite (buf, sizeof (ico_subimage))) { + return false; } // do the actual moving, 0.5kB per iteration @@ -269,9 +274,7 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, return false; fseek (m_file, skip + left - amount + sizeof (ico_subimage), SEEK_SET); - size_t byte_count = fwrite (buf, amount, 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico. (err: %d)", byte_count); + if (!fwrite (buf, amount)) { return false; } } @@ -284,12 +287,8 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&ico.count); } - { - size_t byte_count = fwrite (&ico, sizeof (ico), 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico. (err: %d)", byte_count); - return false; - } + if (!fwrite(ico)) { + return false; } // and finally, update the offsets in subimage headers to point to @@ -306,11 +305,9 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&temp); // roll back 4 bytes, we need to rewrite the value we just read fseek (m_file, -4, SEEK_CUR); - size_t byte_count = fwrite (&temp, sizeof (temp), 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico. (err: %d)", byte_count); - return false; - } + if (!fwrite(temp)) { + return false; + } // skip to the next subimage; subtract 4 bytes because that's how // much we've just written @@ -344,11 +341,9 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&subimg.len); swap_endian (&subimg.ofs); } - size_t byte_count = fwrite (&subimg, 1, sizeof(subimg), m_file); - if (byte_count != sizeof(subimg)) { - error ("Bad file write in ico open. (err: %d)", byte_count); - return false; - } + if (!fwrite(subimg)) { + return false; + } fseek (m_file, m_offset, SEEK_SET); @@ -377,9 +372,7 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&bmi.len); } - size_t byte_count = fwrite (&bmi, sizeof (bmi), 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico open. (err: %d)", byte_count); + if (!fwrite(bmi)) { return false; } @@ -387,9 +380,7 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, char buf[512]; memset (buf, 0, sizeof (buf)); for (int left = bmi.len; left > 0; left -= sizeof (buf)) { - size_t byte_count = fwrite (buf, std::min (left, (int)sizeof (buf)), 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico open. (err: %d)", byte_count); + if (! fwrite (buf, std::min (left, (int)sizeof (buf)))) { return false; } } @@ -485,9 +476,7 @@ ICOOutput::write_scanline (int y, int z, TypeDesc format, break; } - size_t byte_count = fwrite (buf, buff_size, 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico write_scanline (err: %d)", byte_count); + if (!fwrite (buf, buff_size)) { return false; } } @@ -519,11 +508,9 @@ ICOOutput::write_scanline (int y, int z, TypeDesc format, } } - size_t byte_count = fwrite (&buf[0], 1, 1, m_file); - if (byte_count != 1) { - error ("Bad file write in ico write_scanline. (err: %d)", byte_count); - return false; - } + if (!fwrite(buf[0])) { + return false; + } } } } From 3c6f3435452804e5918f176d751398a3742e240d Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 24 Jul 2012 10:40:15 -0400 Subject: [PATCH 17/18] Fixes silly spacing. --- src/targa.imageio/targaoutput.cpp | 82 +++++++++++++++---------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/src/targa.imageio/targaoutput.cpp b/src/targa.imageio/targaoutput.cpp index 11fef76cde..feba22fabd 100644 --- a/src/targa.imageio/targaoutput.cpp +++ b/src/targa.imageio/targaoutput.cpp @@ -226,14 +226,14 @@ TGAOutput::open (const std::string &name, const ImageSpec &userspec, !fwrite(tga.height) || !fwrite(tga.bpp) || !fwrite(tga.attr)) { - return false; + return false; } // dump comment to file, don't bother about null termination if (tga.idlen) { - if (!fwrite(id.c_str(), tga.idlen)) { - return false; + if (!fwrite(id.c_str(), tga.idlen)) { + return false; } } @@ -245,14 +245,14 @@ TGAOutput::open (const std::string &name, const ImageSpec &userspec, bool TGAOutput::close () { - // This call is made a lot: + // This call is made a lot: #define WRITE_TMP_INT(count) { \ - if (!fwrite (tmpint, count)) { \ - return false; \ - } \ - } + if (!fwrite (tmpint, count)) { \ + return false; \ + } \ + } - if (m_file) { + if (m_file) { // write out the TGA 2.0 data fields // FIXME: write out the developer area; according to Larry, @@ -308,13 +308,13 @@ TGAOutput::close () // author std::string tmpstr = m_spec.get_string_attribute ("Artist", ""); - if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { - return false; - } + if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { + return false; + } // fill the rest with zeros for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) { - WRITE_TMP_INT(1); + WRITE_TMP_INT(1); } // image comment @@ -327,7 +327,7 @@ TGAOutput::close () // on line breaks, fill the remainder of the line with zeros if (p[pos] == '\n') { while ((w + 1) % 81 != 0) { - WRITE_TMP_INT(1); + WRITE_TMP_INT(1); w++; } @@ -339,13 +339,13 @@ TGAOutput::close () } // null-terminate each line if ((w + 1) % 81 == 0) { - WRITE_TMP_INT(1); + WRITE_TMP_INT(1); w++; } } // fill the rest with zeros for (; w < 324; w++) { - WRITE_TMP_INT(1); + WRITE_TMP_INT(1); } } @@ -372,22 +372,22 @@ TGAOutput::close () !fwrite(h) || !fwrite(i) || !fwrite(s)) { - return false; + return false; } } // job ID tmpstr = m_spec.get_string_attribute ("DocumentName", ""); - if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { - return false; - } + if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { + return false; + } // fill the rest with zeros for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) { - if (!fwrite (tmpint) || - !fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { - return false; - } + if (!fwrite (tmpint) || + !fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { + return false; + } } // job time tmpstr = m_spec.get_string_attribute ("targa:JobTime", ""); @@ -405,16 +405,16 @@ TGAOutput::close () if (!fwrite(h) || !fwrite(m) || !fwrite(s)) { - return false; + return false; } } // software ID - we advertise ourselves tmpstr = OIIO_INTRO_STRING; - if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { - return false; - } + if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { + return false; + } // fill the rest with zeros for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) WRITE_TMP_INT(1); @@ -453,8 +453,8 @@ TGAOutput::close () tmpint = 0; } else { // just dump two zeros in there - WRITE_TMP_INT(2); - WRITE_TMP_INT(2); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); } } @@ -476,8 +476,8 @@ TGAOutput::close () tmpint = 0; } else { // just dump two zeros in there - WRITE_TMP_INT(2); - WRITE_TMP_INT(2); + WRITE_TMP_INT(2); + WRITE_TMP_INT(2); } } @@ -511,8 +511,8 @@ TGAOutput::close () if (!fwrite(&foot.ofs_ext) || !fwrite(&foot.ofs_dev) || !fwrite(&foot.signature)) { - return false; - } + return false; + } // close the stream fclose (m_file); @@ -539,8 +539,8 @@ TGAOutput::flush_rlp (unsigned char *buf, int size) // write packet pixel if (!fwrite(h) || !fwrite (buf, m_spec.nchannels)) { // do something intelligent? - return; - } + return; + } } @@ -562,7 +562,7 @@ TGAOutput::flush_rawp (unsigned char *& src, int size, int start) if (n <= 2) { // 1- and 2-channels can write directly if (!fwrite (src+start, n)) { - return; + return; } } else { // 3- and 4-channel must swap red and blue @@ -718,7 +718,7 @@ TGAOutput::write_scanline (int y, int z, TypeDesc format, if (n <= 2) { // 1- and 2-channels can write directly if (!fwrite (bdata, n, w)) { - return false; + return false; } } else { // 3- and 4-channels must swap R and B @@ -727,9 +727,9 @@ TGAOutput::write_scanline (int y, int z, TypeDesc format, for (int x = 0; x < m_spec.width; x++) std::swap (buf[x*n], buf[x*n+2]); - if (!fwrite (&buf[0], n, w)) { - return false; - } + if (!fwrite (&buf[0], n, w)) { + return false; + } } } From 44aa3f09671becef782e4cf43334e54a04fe4c81 Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 24 Jul 2012 10:41:46 -0400 Subject: [PATCH 18/18] More spacing. --- src/ico.imageio/icooutput.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ico.imageio/icooutput.cpp b/src/ico.imageio/icooutput.cpp index c3a3b58dd3..9aabe53bfc 100644 --- a/src/ico.imageio/icooutput.cpp +++ b/src/ico.imageio/icooutput.cpp @@ -274,9 +274,9 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, return false; fseek (m_file, skip + left - amount + sizeof (ico_subimage), SEEK_SET); - if (!fwrite (buf, amount)) { - return false; - } + if (!fwrite (buf, amount)) { + return false; + } } // update header @@ -372,17 +372,17 @@ ICOOutput::open (const std::string &name, const ImageSpec &userspec, swap_endian (&bmi.len); } - if (!fwrite(bmi)) { - return false; - } + if (!fwrite(bmi)) { + return false; + } // append null data so that we don't seek beyond eof in write_scanline char buf[512]; memset (buf, 0, sizeof (buf)); for (int left = bmi.len; left > 0; left -= sizeof (buf)) { - if (! fwrite (buf, std::min (left, (int)sizeof (buf)))) { - return false; - } + if (! fwrite (buf, std::min (left, (int)sizeof (buf)))) { + return false; + } } fseek (m_file, m_offset + sizeof (bmi), SEEK_SET); } @@ -477,7 +477,7 @@ ICOOutput::write_scanline (int y, int z, TypeDesc format, } if (!fwrite (buf, buff_size)) { - return false; + return false; } }