From 889951e59968b01352ea51c0b2166415b573946c Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 10 Jan 2013 10:12:06 -0500 Subject: [PATCH 1/5] Fixes #1650 where using a file-like object on Python 3 fails. Use npy_PyFile_* compatibility methods instead of rolling it ourselves. --- src/_png.cpp | 104 +++++++++++++++++++++++++-------------------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/src/_png.cpp b/src/_png.cpp index 23ee5980b79a..5c486ad87af4 100644 --- a/src/_png.cpp +++ b/src/_png.cpp @@ -27,6 +27,7 @@ #include "CXX/Extensions.hxx" #include "numpy/arrayobject.h" #include "mplutils.h" +#include "numpy/npy_3kcompat.h" // As reported in [3082058] build _png.so on aix #ifdef _AIX @@ -104,6 +105,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args) FILE *fp = NULL; bool close_file = false; + bool close_dup_file = false; Py::Object buffer_obj = Py::Object(args[0]); PyObject* buffer = buffer_obj.ptr(); if (!PyObject_CheckReadBuffer(buffer)) @@ -128,41 +130,33 @@ Py::Object _png_module::write_png(const Py::Tuple& args) } Py::Object py_fileobj = Py::Object(args[3]); -#if PY3K - int fd = PyObject_AsFileDescriptor(py_fileobj.ptr()); - PyErr_Clear(); -#endif + PyObject* py_file = NULL; if (py_fileobj.isString()) { - std::string fileName = Py::String(py_fileobj); - const char *file_name = fileName.c_str(); - if ((fp = fopen(file_name, "wb")) == NULL) - { - throw Py::RuntimeError( - Printf("Could not open file %s", file_name).str()); + if ((py_file = npy_PyFile_OpenFile(py_file, (char *)"w")) == NULL) { + throw Py::Exception(); } close_file = true; } -#if PY3K - else if (fd != -1) + else { - fp = fdopen(fd, "w"); + py_file = py_fileobj.ptr(); } -#else - else if (PyFile_CheckExact(py_fileobj.ptr())) + + if ((fp = npy_PyFile_Dup(py_file, (char *)"w"))) { - fp = PyFile_AsFile(py_fileobj.ptr()); + close_dup_file = true; } -#endif else { PyObject* write_method = PyObject_GetAttrString( - py_fileobj.ptr(), "write"); + py_file, "write"); if (!(write_method && PyCallable_Check(write_method))) { Py_XDECREF(write_method); throw Py::TypeError( - "Object does not appear to be a 8-bit string path or a Python file-like object"); + "Object does not appear to be a 8-bit string path or " + "a Python file-like object"); } Py_XDECREF(write_method); } @@ -205,7 +199,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args) } else { - png_set_write_fn(png_ptr, (void*)py_fileobj.ptr(), + png_set_write_fn(png_ptr, (void*)py_file, &write_png_data, &flush_png_data); } png_set_IHDR(png_ptr, info_ptr, @@ -241,9 +235,16 @@ Py::Object _png_module::write_png(const Py::Tuple& args) png_destroy_write_struct(&png_ptr, &info_ptr); } delete [] row_pointers; - if (fp && close_file) + + if (close_dup_file) + { + npy_PyFile_DupClose(py_file, fp); + } + + if (close_file) { - fclose(fp); + npy_PyFile_CloseFile(py_file); + Py_DECREF(py_file); } /* Changed calls to png_destroy_write_struct to follow http://www.libpng.org/pub/png/libpng-manual.txt. @@ -254,15 +255,15 @@ Py::Object _png_module::write_png(const Py::Tuple& args) png_destroy_write_struct(&png_ptr, &info_ptr); delete [] row_pointers; -#if PY3K - if (fp) + if (close_dup_file) { - fflush(fp); + npy_PyFile_DupClose(py_file, fp); } -#endif - if (fp && close_file) + + if (close_file) { - fclose(fp); + npy_PyFile_CloseFile(py_file); + Py_DECREF(py_file); } if (PyErr_Occurred()) { @@ -306,40 +307,32 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, png_byte header[8]; // 8 is the maximum size that can be checked FILE* fp = NULL; bool close_file = false; - -#if PY3K - int fd = PyObject_AsFileDescriptor(py_fileobj.ptr()); - PyErr_Clear(); -#endif + bool close_dup_file = false; + PyObject *py_file = NULL; if (py_fileobj.isString()) { - std::string fileName = Py::String(py_fileobj); - const char *file_name = fileName.c_str(); - if ((fp = fopen(file_name, "rb")) == NULL) - { - throw Py::RuntimeError( - Printf("Could not open file %s for reading", file_name).str()); + if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"r")) == NULL) { + throw Py::Exception(); } close_file = true; + } else { + py_file = py_fileobj.ptr(); } -#if PY3K - else if (fd != -1) { - fp = fdopen(fd, "r"); - } -#else - else if (PyFile_CheckExact(py_fileobj.ptr())) + + if ((fp = npy_PyFile_Dup(py_file, "r"))) { - fp = PyFile_AsFile(py_fileobj.ptr()); + close_dup_file = true; } -#endif else { - PyObject* read_method = PyObject_GetAttrString(py_fileobj.ptr(), "read"); + PyObject* read_method = PyObject_GetAttrString(py_file, "read"); if (!(read_method && PyCallable_Check(read_method))) { Py_XDECREF(read_method); - throw Py::TypeError("Object does not appear to be a 8-bit string path or a Python file-like object"); + throw Py::TypeError( + "Object does not appear to be a 8-bit string path or a Python " + "file-like object"); } Py_XDECREF(read_method); } @@ -354,7 +347,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, } else { - _read_png_data(py_fileobj.ptr(), header, 8); + _read_png_data(py_file, header, 8); } if (png_sig_cmp(header, 0, 8)) { @@ -390,7 +383,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, } else { - png_set_read_fn(png_ptr, (void*)py_fileobj.ptr(), &read_png_data); + png_set_read_fn(png_ptr, (void*)py_file, &read_png_data); } png_set_sig_bytes(png_ptr, 8); png_read_info(png_ptr, info_ptr); @@ -572,10 +565,17 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, #else png_destroy_read_struct(&png_ptr, &info_ptr, png_infopp_NULL); #endif + if (close_dup_file) + { + npy_PyFile_DupClose(py_file, fp); + } + if (close_file) { - fclose(fp); + npy_PyFile_CloseFile(py_file); + Py_DECREF(py_file); } + for (row = 0; row < height; row++) { delete [] row_pointers[row]; From cd6c8c932ea3fc768b145a9ad053bf05b2f1b62d Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Fri, 11 Jan 2013 10:29:13 -0500 Subject: [PATCH 2/5] Clear exceptions if PyFile_Dup() fails -- we have a way to handle "non-real" file objects. --- src/_png.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/_png.cpp b/src/_png.cpp index 5c486ad87af4..5a8e5888ad23 100644 --- a/src/_png.cpp +++ b/src/_png.cpp @@ -149,6 +149,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args) } else { + PyErr_Clear(); PyObject* write_method = PyObject_GetAttrString( py_file, "write"); if (!(write_method && PyCallable_Check(write_method))) @@ -326,6 +327,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, } else { + PyErr_Clear(); PyObject* read_method = PyObject_GetAttrString(py_file, "read"); if (!(read_method && PyCallable_Check(read_method))) { From bee6479a6d9639bd32e83e73b3fd8408af342b19 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Fri, 11 Jan 2013 11:11:57 -0500 Subject: [PATCH 3/5] Copy over some of the Numpy file-handing compatibility functions. --- src/_png.cpp | 4 +- src/file_compat.h | 135 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 src/file_compat.h diff --git a/src/_png.cpp b/src/_png.cpp index 5a8e5888ad23..0253845947b5 100644 --- a/src/_png.cpp +++ b/src/_png.cpp @@ -27,7 +27,7 @@ #include "CXX/Extensions.hxx" #include "numpy/arrayobject.h" #include "mplutils.h" -#include "numpy/npy_3kcompat.h" +#include "file_compat.h" // As reported in [3082058] build _png.so on aix #ifdef _AIX @@ -133,7 +133,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args) PyObject* py_file = NULL; if (py_fileobj.isString()) { - if ((py_file = npy_PyFile_OpenFile(py_file, (char *)"w")) == NULL) { + if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"w")) == NULL) { throw Py::Exception(); } close_file = true; diff --git a/src/file_compat.h b/src/file_compat.h new file mode 100644 index 000000000000..fbb545d5665e --- /dev/null +++ b/src/file_compat.h @@ -0,0 +1,135 @@ +#ifndef __FILE_COMPAT_H__ +#define __FILE_COMPAT_H__ + +#include "numpy/npy_3kcompat.h" + +#if NPY_API_VERSION < 0x4 /* corresponds to Numpy 1.5 */ +/* + * PyFile_* compatibility + */ +#if defined(NPY_PY3K) + +/* + * Get a FILE* handle to the file represented by the Python object + */ +static NPY_INLINE FILE* +npy_PyFile_Dup(PyObject *file, char *mode) +{ + int fd, fd2; + PyObject *ret, *os; + Py_ssize_t pos; + FILE *handle; + /* Flush first to ensure things end up in the file in the correct order */ + ret = PyObject_CallMethod(file, "flush", ""); + if (ret == NULL) { + return NULL; + } + Py_DECREF(ret); + fd = PyObject_AsFileDescriptor(file); + if (fd == -1) { + return NULL; + } + os = PyImport_ImportModule("os"); + if (os == NULL) { + return NULL; + } + ret = PyObject_CallMethod(os, "dup", "i", fd); + Py_DECREF(os); + if (ret == NULL) { + return NULL; + } + fd2 = PyNumber_AsSsize_t(ret, NULL); + Py_DECREF(ret); +#ifdef _WIN32 + handle = _fdopen(fd2, mode); +#else + handle = fdopen(fd2, mode); +#endif + if (handle == NULL) { + PyErr_SetString(PyExc_IOError, + "Getting a FILE* from a Python file object failed"); + } + ret = PyObject_CallMethod(file, "tell", ""); + if (ret == NULL) { + fclose(handle); + return NULL; + } + pos = PyNumber_AsSsize_t(ret, PyExc_OverflowError); + Py_DECREF(ret); + if (PyErr_Occurred()) { + fclose(handle); + return NULL; + } + npy_fseek(handle, pos, SEEK_SET); + return handle; +} + +/* + * Close the dup-ed file handle, and seek the Python one to the current position + */ +static NPY_INLINE int +npy_PyFile_DupClose(PyObject *file, FILE* handle) +{ + PyObject *ret; + Py_ssize_t position; + position = npy_ftell(handle); + fclose(handle); + + ret = PyObject_CallMethod(file, "seek", NPY_SSIZE_T_PYFMT "i", position, 0); + if (ret == NULL) { + return -1; + } + Py_DECREF(ret); + return 0; +} + +static NPY_INLINE int +npy_PyFile_Check(PyObject *file) +{ + int fd; + fd = PyObject_AsFileDescriptor(file); + if (fd == -1) { + PyErr_Clear(); + return 0; + } + return 1; +} + +#else + +#define npy_PyFile_Dup(file, mode) PyFile_AsFile(file) +#define npy_PyFile_DupClose(file, handle) (0) + +#endif + +static NPY_INLINE PyObject* +npy_PyFile_OpenFile(PyObject *filename, const char *mode) +{ + PyObject *open; + open = PyDict_GetItemString(PyEval_GetBuiltins(), "open"); + if (open == NULL) { + return NULL; + } + return PyObject_CallFunction(open, "Os", filename, mode); +} + +#endif /* NPY_API_VERSION < 0x4 */ + +#if NPY_API_VERSION < 0x7 /* corresponds to Numpy 1.7 */ + +static NPY_INLINE int +npy_PyFile_CloseFile(PyObject *file) +{ + PyObject *ret; + + ret = PyObject_CallMethod(file, "close", NULL); + if (ret == NULL) { + return -1; + } + Py_DECREF(ret); + return 0; +} + +#endif /* NPY_API_VERSION < 0x7 */ + +#endif /* ifndef __FILE_COMPAT_H__ */ From c3d39b5f3bdab7e827d6e616c0f474d058fa6281 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Fri, 11 Jan 2013 11:12:09 -0500 Subject: [PATCH 4/5] Update backend_agg to use the Numpy file-handling compatibility functions. --- src/_backend_agg.cpp | 49 ++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/_backend_agg.cpp b/src/_backend_agg.cpp index 6116dd7eeb66..6c256c60051a 100644 --- a/src/_backend_agg.cpp +++ b/src/_backend_agg.cpp @@ -39,6 +39,7 @@ #include "numpy/arrayobject.h" #include "agg_py_transforms.h" +#include "file_compat.h" #ifndef M_PI #define M_PI 3.14159265358979323846 @@ -2028,44 +2029,42 @@ RendererAgg::write_rgba(const Py::Tuple& args) FILE *fp = NULL; Py::Object py_fileobj = Py::Object(args[0]); - - #if PY3K - int fd = PyObject_AsFileDescriptor(py_fileobj.ptr()); - PyErr_Clear(); - #endif + PyObject* py_file = NULL; + bool close_file = false; if (py_fileobj.isString()) { - std::string fileName = Py::String(py_fileobj); - const char *file_name = fileName.c_str(); - if ((fp = fopen(file_name, "wb")) == NULL) - throw Py::RuntimeError( - Printf("Could not open file %s", file_name).str()); - if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES) - { - fclose(fp); - throw Py::RuntimeError( - Printf("Error writing to file %s", file_name).str()); + if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"w")) == NULL) { + throw Py::Exception(); } + close_file = true; } - #if PY3K - else if (fd != -1) + else { - if (write(fd, pixBuffer, NUMBYTES) != (ssize_t)NUMBYTES) - { - throw Py::RuntimeError("Error writing to file"); - } + py_file = py_fileobj.ptr(); } - #else - else if (PyFile_CheckExact(py_fileobj.ptr())) + + if ((fp = npy_PyFile_Dup(py_file, (char *)"w"))) { - fp = PyFile_AsFile(py_fileobj.ptr()); if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES) { + npy_PyFile_DupClose(py_file, fp); + + if (close_file) { + npy_PyFile_CloseFile(py_file); + Py_DECREF(py_file); + } + throw Py::RuntimeError("Error writing to file"); } + + npy_PyFile_DupClose(py_file, fp); + + if (close_file) { + npy_PyFile_CloseFile(py_file); + Py_DECREF(py_file); + } } - #endif else { PyObject* write_method = PyObject_GetAttrString(py_fileobj.ptr(), From dda9b7ab3e4eacea152f9cd0ba8d8f5816a89e15 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Fri, 11 Jan 2013 11:15:50 -0500 Subject: [PATCH 5/5] Open files in binary mode. --- src/_backend_agg.cpp | 4 ++-- src/_png.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_backend_agg.cpp b/src/_backend_agg.cpp index 6c256c60051a..fb4a61f8a564 100644 --- a/src/_backend_agg.cpp +++ b/src/_backend_agg.cpp @@ -2034,7 +2034,7 @@ RendererAgg::write_rgba(const Py::Tuple& args) if (py_fileobj.isString()) { - if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"w")) == NULL) { + if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"wb")) == NULL) { throw Py::Exception(); } close_file = true; @@ -2044,7 +2044,7 @@ RendererAgg::write_rgba(const Py::Tuple& args) py_file = py_fileobj.ptr(); } - if ((fp = npy_PyFile_Dup(py_file, (char *)"w"))) + if ((fp = npy_PyFile_Dup(py_file, (char *)"wb"))) { if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES) { diff --git a/src/_png.cpp b/src/_png.cpp index 0253845947b5..9f23b214f7e7 100644 --- a/src/_png.cpp +++ b/src/_png.cpp @@ -133,7 +133,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args) PyObject* py_file = NULL; if (py_fileobj.isString()) { - if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"w")) == NULL) { + if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"wb")) == NULL) { throw Py::Exception(); } close_file = true; @@ -143,7 +143,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args) py_file = py_fileobj.ptr(); } - if ((fp = npy_PyFile_Dup(py_file, (char *)"w"))) + if ((fp = npy_PyFile_Dup(py_file, (char *)"wb"))) { close_dup_file = true; } @@ -313,7 +313,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, if (py_fileobj.isString()) { - if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"r")) == NULL) { + if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"rb")) == NULL) { throw Py::Exception(); } close_file = true; @@ -321,7 +321,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result, py_file = py_fileobj.ptr(); } - if ((fp = npy_PyFile_Dup(py_file, "r"))) + if ((fp = npy_PyFile_Dup(py_file, "rb"))) { close_dup_file = true; }