Move the png open and destroy functions into a class
The open_png() function used to open the png file but didn't close it;
and this caused the leak of fd. However, we cannot close the file inside
open_png() because the png file needs to remain open until the outer
function finishes parsing the file and destroys the png struct.
This CL addresses this issue by implementing a PngReader class to handle
the creation/destruction of the png struct.
Bug: 67010912
Test: Run graphic tests; also run locale tests and check fd.
Change-Id: I9a803b3cd8c16f16a9ffe8f0acc7fe0f42e95eb0
diff --git a/minui/resources.cpp b/minui/resources.cpp
index 756f29d..837f5eb 100644
--- a/minui/resources.cpp
+++ b/minui/resources.cpp
@@ -25,10 +25,12 @@
#include <sys/types.h>
#include <unistd.h>
+#include <memory>
#include <regex>
#include <string>
#include <vector>
+#include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <png.h>
@@ -46,89 +48,126 @@
return surface;
}
-static int open_png(const char* name, png_structp* png_ptr, png_infop* info_ptr,
- png_uint_32* width, png_uint_32* height, png_byte* channels) {
- char resPath[256];
- unsigned char header[8];
- int result = 0;
- int color_type, bit_depth;
- size_t bytesRead;
+// This class handles the png file parsing. It also holds the ownership of the png pointer and the
+// opened file pointer. Both will be destroyed/closed when this object goes out of scope.
+class PngHandler {
+ public:
+ PngHandler(const std::string& name);
- snprintf(resPath, sizeof(resPath)-1, "/res/images/%s.png", name);
- resPath[sizeof(resPath)-1] = '\0';
- FILE* fp = fopen(resPath, "rbe");
- if (fp == NULL) {
- result = -1;
- goto exit;
- }
+ ~PngHandler();
- bytesRead = fread(header, 1, sizeof(header), fp);
- if (bytesRead != sizeof(header)) {
- result = -2;
- goto exit;
- }
+ png_uint_32 width() const {
+ return width_;
+ }
- if (png_sig_cmp(header, 0, sizeof(header))) {
- result = -3;
- goto exit;
- }
+ png_uint_32 height() const {
+ return height_;
+ }
- *png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
- if (!*png_ptr) {
- result = -4;
- goto exit;
- }
+ png_byte channels() const {
+ return channels_;
+ }
- *info_ptr = png_create_info_struct(*png_ptr);
- if (!*info_ptr) {
- result = -5;
- goto exit;
- }
+ png_structp png_ptr() const {
+ return png_ptr_;
+ }
- if (setjmp(png_jmpbuf(*png_ptr))) {
- result = -6;
- goto exit;
- }
+ png_infop info_ptr() const {
+ return info_ptr_;
+ }
- png_init_io(*png_ptr, fp);
- png_set_sig_bytes(*png_ptr, sizeof(header));
- png_read_info(*png_ptr, *info_ptr);
+ int error_code() const {
+ return error_code_;
+ };
- png_get_IHDR(*png_ptr, *info_ptr, width, height, &bit_depth,
- &color_type, NULL, NULL, NULL);
+ operator bool() const {
+ return error_code_ == 0;
+ }
- *channels = png_get_channels(*png_ptr, *info_ptr);
+ private:
+ png_structp png_ptr_{ nullptr };
+ png_infop info_ptr_{ nullptr };
+ png_uint_32 width_;
+ png_uint_32 height_;
+ png_byte channels_;
- if (bit_depth == 8 && *channels == 3 && color_type == PNG_COLOR_TYPE_RGB) {
- // 8-bit RGB images: great, nothing to do.
- } else if (bit_depth <= 8 && *channels == 1 && color_type == PNG_COLOR_TYPE_GRAY) {
- // 1-, 2-, 4-, or 8-bit gray images: expand to 8-bit gray.
- png_set_expand_gray_1_2_4_to_8(*png_ptr);
- } else if (bit_depth <= 8 && *channels == 1 && color_type == PNG_COLOR_TYPE_PALETTE) {
- // paletted images: expand to 8-bit RGB. Note that we DON'T
- // currently expand the tRNS chunk (if any) to an alpha
- // channel, because minui doesn't support alpha channels in
- // general.
- png_set_palette_to_rgb(*png_ptr);
- *channels = 3;
- } else {
- fprintf(stderr, "minui doesn't support PNG depth %d channels %d color_type %d\n",
- bit_depth, *channels, color_type);
- result = -7;
- goto exit;
- }
+ // The |error_code_| is set to a negative value if an error occurs when opening the png file.
+ int error_code_;
+ // After initialization, we'll keep the file pointer open before destruction of PngHandler.
+ std::unique_ptr<FILE, decltype(&fclose)> png_fp_;
+};
- return result;
+PngHandler::PngHandler(const std::string& name) : error_code_(0), png_fp_(nullptr, fclose) {
+ std::string res_path = android::base::StringPrintf("/res/images/%s.png", name.c_str());
+ png_fp_.reset(fopen(res_path.c_str(), "rbe"));
+ if (!png_fp_) {
+ error_code_ = -1;
+ return;
+ }
- exit:
- if (result < 0) {
- png_destroy_read_struct(png_ptr, info_ptr, NULL);
- }
- if (fp != NULL) {
- fclose(fp);
- }
+ unsigned char header[8];
+ size_t bytesRead = fread(header, 1, sizeof(header), png_fp_.get());
+ if (bytesRead != sizeof(header)) {
+ error_code_ = -2;
+ return;
+ }
- return result;
+ if (png_sig_cmp(header, 0, sizeof(header))) {
+ error_code_ = -3;
+ return;
+ }
+
+ png_ptr_ = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
+ if (!png_ptr_) {
+ error_code_ = -4;
+ return;
+ }
+
+ info_ptr_ = png_create_info_struct(png_ptr_);
+ if (!info_ptr_) {
+ error_code_ = -5;
+ return;
+ }
+
+ if (setjmp(png_jmpbuf(png_ptr_))) {
+ error_code_ = -6;
+ return;
+ }
+
+ png_init_io(png_ptr_, png_fp_.get());
+ png_set_sig_bytes(png_ptr_, sizeof(header));
+ png_read_info(png_ptr_, info_ptr_);
+
+ int color_type;
+ int bit_depth;
+ png_get_IHDR(png_ptr_, info_ptr_, &width_, &height_, &bit_depth, &color_type, nullptr, nullptr,
+ nullptr);
+
+ channels_ = png_get_channels(png_ptr_, info_ptr_);
+
+ if (bit_depth == 8 && channels_ == 3 && color_type == PNG_COLOR_TYPE_RGB) {
+ // 8-bit RGB images: great, nothing to do.
+ } else if (bit_depth <= 8 && channels_ == 1 && color_type == PNG_COLOR_TYPE_GRAY) {
+ // 1-, 2-, 4-, or 8-bit gray images: expand to 8-bit gray.
+ png_set_expand_gray_1_2_4_to_8(png_ptr_);
+ } else if (bit_depth <= 8 && channels_ == 1 && color_type == PNG_COLOR_TYPE_PALETTE) {
+ // paletted images: expand to 8-bit RGB. Note that we DON'T
+ // currently expand the tRNS chunk (if any) to an alpha
+ // channel, because minui doesn't support alpha channels in
+ // general.
+ png_set_palette_to_rgb(png_ptr_);
+ channels_ = 3;
+ } else {
+ fprintf(stderr, "minui doesn't support PNG depth %d channels %d color_type %d\n", bit_depth,
+ channels_, color_type);
+ error_code_ = -7;
+ }
+}
+
+PngHandler::~PngHandler() {
+ if (png_ptr_) {
+ png_destroy_read_struct(&png_ptr_, &info_ptr_, nullptr);
+ }
}
// "display" surfaces are transformed into the framebuffer's required
@@ -198,178 +237,152 @@
}
int res_create_display_surface(const char* name, GRSurface** pSurface) {
- GRSurface* surface = NULL;
- int result = 0;
- png_structp png_ptr = NULL;
- png_infop info_ptr = NULL;
- png_uint_32 width, height;
- png_byte channels;
- unsigned char* p_row;
- unsigned int y;
+ *pSurface = nullptr;
- *pSurface = NULL;
+ PngHandler png_handler(name);
+ if (!png_handler) return png_handler.error_code();
- result = open_png(name, &png_ptr, &info_ptr, &width, &height, &channels);
- if (result < 0) return result;
+ png_structp png_ptr = png_handler.png_ptr();
+ png_uint_32 width = png_handler.width();
+ png_uint_32 height = png_handler.height();
- surface = init_display_surface(width, height);
- if (surface == NULL) {
- result = -8;
- goto exit;
- }
+ GRSurface* surface = init_display_surface(width, height);
+ if (!surface) {
+ return -8;
+ }
#if defined(RECOVERY_ABGR) || defined(RECOVERY_BGRA)
- png_set_bgr(png_ptr);
+ png_set_bgr(png_ptr);
#endif
- p_row = static_cast<unsigned char*>(malloc(width * 4));
- for (y = 0; y < height; ++y) {
- png_read_row(png_ptr, p_row, NULL);
- transform_rgb_to_draw(p_row, surface->data + y * surface->row_bytes, channels, width);
- }
- free(p_row);
+ for (png_uint_32 y = 0; y < height; ++y) {
+ std::vector<unsigned char> p_row(width * 4);
+ png_read_row(png_ptr, p_row.data(), nullptr);
+ transform_rgb_to_draw(p_row.data(), surface->data + y * surface->row_bytes,
+ png_handler.channels(), width);
+ }
- *pSurface = surface;
+ *pSurface = surface;
- exit:
- png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
- if (result < 0 && surface != NULL) free(surface);
- return result;
+ return 0;
}
int res_create_multi_display_surface(const char* name, int* frames, int* fps,
- GRSurface*** pSurface) {
- GRSurface** surface = NULL;
- int result = 0;
- png_structp png_ptr = NULL;
- png_infop info_ptr = NULL;
- png_uint_32 width, height;
- png_byte channels;
- png_textp text;
- int num_text;
- unsigned char* p_row;
- unsigned int y;
+ GRSurface*** pSurface) {
+ *pSurface = nullptr;
+ *frames = -1;
- *pSurface = NULL;
- *frames = -1;
+ PngHandler png_handler(name);
+ if (!png_handler) return png_handler.error_code();
- result = open_png(name, &png_ptr, &info_ptr, &width, &height, &channels);
- if (result < 0) return result;
+ png_structp png_ptr = png_handler.png_ptr();
+ png_uint_32 width = png_handler.width();
+ png_uint_32 height = png_handler.height();
- *frames = 1;
- *fps = 20;
- if (png_get_text(png_ptr, info_ptr, &text, &num_text)) {
- for (int i = 0; i < num_text; ++i) {
- if (text[i].key && strcmp(text[i].key, "Frames") == 0 && text[i].text) {
- *frames = atoi(text[i].text);
- } else if (text[i].key && strcmp(text[i].key, "FPS") == 0 && text[i].text) {
- *fps = atoi(text[i].text);
- }
- }
- printf(" found frames = %d\n", *frames);
- printf(" found fps = %d\n", *fps);
+ *frames = 1;
+ *fps = 20;
+ png_textp text;
+ int num_text;
+ if (png_get_text(png_ptr, png_handler.info_ptr(), &text, &num_text)) {
+ for (int i = 0; i < num_text; ++i) {
+ if (text[i].key && strcmp(text[i].key, "Frames") == 0 && text[i].text) {
+ *frames = atoi(text[i].text);
+ } else if (text[i].key && strcmp(text[i].key, "FPS") == 0 && text[i].text) {
+ *fps = atoi(text[i].text);
+ }
}
+ printf(" found frames = %d\n", *frames);
+ printf(" found fps = %d\n", *fps);
+ }
- if (*frames <= 0 || *fps <= 0) {
- printf("bad number of frames (%d) and/or FPS (%d)\n", *frames, *fps);
- result = -10;
- goto exit;
- }
+ int result = 0;
+ GRSurface** surface = nullptr;
+ if (*frames <= 0 || *fps <= 0) {
+ printf("bad number of frames (%d) and/or FPS (%d)\n", *frames, *fps);
+ result = -10;
+ goto exit;
+ }
- if (height % *frames != 0) {
- printf("bad height (%d) for frame count (%d)\n", height, *frames);
- result = -9;
- goto exit;
- }
+ if (height % *frames != 0) {
+ printf("bad height (%d) for frame count (%d)\n", height, *frames);
+ result = -9;
+ goto exit;
+ }
- surface = static_cast<GRSurface**>(calloc(*frames, sizeof(GRSurface*)));
- if (surface == NULL) {
- result = -8;
- goto exit;
+ surface = static_cast<GRSurface**>(calloc(*frames, sizeof(GRSurface*)));
+ if (!surface) {
+ result = -8;
+ goto exit;
+ }
+ for (int i = 0; i < *frames; ++i) {
+ surface[i] = init_display_surface(width, height / *frames);
+ if (!surface[i]) {
+ result = -8;
+ goto exit;
}
- for (int i = 0; i < *frames; ++i) {
- surface[i] = init_display_surface(width, height / *frames);
- if (surface[i] == NULL) {
- result = -8;
- goto exit;
- }
- }
+ }
#if defined(RECOVERY_ABGR) || defined(RECOVERY_BGRA)
- png_set_bgr(png_ptr);
+ png_set_bgr(png_ptr);
#endif
- p_row = static_cast<unsigned char*>(malloc(width * 4));
- for (y = 0; y < height; ++y) {
- png_read_row(png_ptr, p_row, NULL);
- int frame = y % *frames;
- unsigned char* out_row = surface[frame]->data +
- (y / *frames) * surface[frame]->row_bytes;
- transform_rgb_to_draw(p_row, out_row, channels, width);
- }
- free(p_row);
+ for (png_uint_32 y = 0; y < height; ++y) {
+ std::vector<unsigned char> p_row(width * 4);
+ png_read_row(png_ptr, p_row.data(), nullptr);
+ int frame = y % *frames;
+ unsigned char* out_row = surface[frame]->data + (y / *frames) * surface[frame]->row_bytes;
+ transform_rgb_to_draw(p_row.data(), out_row, png_handler.channels(), width);
+ }
- *pSurface = surface;
+ *pSurface = surface;
exit:
- png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
-
- if (result < 0) {
- if (surface) {
- for (int i = 0; i < *frames; ++i) {
- free(surface[i]);
- }
- free(surface);
- }
+ if (result < 0) {
+ if (surface) {
+ for (int i = 0; i < *frames; ++i) {
+ free(surface[i]);
+ }
+ free(surface);
}
- return result;
+ }
+ return result;
}
int res_create_alpha_surface(const char* name, GRSurface** pSurface) {
- GRSurface* surface = NULL;
- int result = 0;
- png_structp png_ptr = NULL;
- png_infop info_ptr = NULL;
- png_uint_32 width, height;
- png_byte channels;
+ *pSurface = nullptr;
- *pSurface = NULL;
+ PngHandler png_handler(name);
+ if (!png_handler) return png_handler.error_code();
- result = open_png(name, &png_ptr, &info_ptr, &width, &height, &channels);
- if (result < 0) return result;
+ if (png_handler.channels() != 1) {
+ return -7;
+ }
- if (channels != 1) {
- result = -7;
- goto exit;
- }
+ png_structp png_ptr = png_handler.png_ptr();
+ png_uint_32 width = png_handler.width();
+ png_uint_32 height = png_handler.height();
- surface = malloc_surface(width * height);
- if (surface == NULL) {
- result = -8;
- goto exit;
- }
- surface->width = width;
- surface->height = height;
- surface->row_bytes = width;
- surface->pixel_bytes = 1;
+ GRSurface* surface = malloc_surface(width * height);
+ if (!surface) {
+ return -8;
+ }
+ surface->width = width;
+ surface->height = height;
+ surface->row_bytes = width;
+ surface->pixel_bytes = 1;
#if defined(RECOVERY_ABGR) || defined(RECOVERY_BGRA)
- png_set_bgr(png_ptr);
+ png_set_bgr(png_ptr);
#endif
- unsigned char* p_row;
- unsigned int y;
- for (y = 0; y < height; ++y) {
- p_row = surface->data + y * surface->row_bytes;
- png_read_row(png_ptr, p_row, NULL);
- }
+ for (png_uint_32 y = 0; y < height; ++y) {
+ unsigned char* p_row = surface->data + y * surface->row_bytes;
+ png_read_row(png_ptr, p_row, nullptr);
+ }
- *pSurface = surface;
+ *pSurface = surface;
- exit:
- png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
- if (result < 0 && surface != NULL) free(surface);
- return result;
+ return 0;
}
// This function tests if a locale string stored in PNG (prefix) matches
@@ -397,109 +410,89 @@
}
std::vector<std::string> get_locales_in_png(const std::string& png_name) {
- png_structp png_ptr = nullptr;
- png_infop info_ptr = nullptr;
- png_uint_32 width, height;
- png_byte channels;
-
- int status = open_png(png_name.c_str(), &png_ptr, &info_ptr, &width, &height, &channels);
- if (status < 0) {
- printf("Failed to open %s\n", png_name.c_str());
+ PngHandler png_handler(png_name);
+ if (!png_handler) {
+ printf("Failed to open %s, error: %d\n", png_name.c_str(), png_handler.error_code());
return {};
}
- if (channels != 1) {
- printf("Expect input png to have 1 data channel, this file has %d\n", channels);
- png_destroy_read_struct(&png_ptr, &info_ptr, nullptr);
+ if (png_handler.channels() != 1) {
+ printf("Expect input png to have 1 data channel, this file has %d\n", png_handler.channels());
return {};
}
std::vector<std::string> result;
- std::vector<unsigned char> row(width);
- for (png_uint_32 y = 0; y < height; ++y) {
- png_read_row(png_ptr, row.data(), nullptr);
+ std::vector<unsigned char> row(png_handler.width());
+ for (png_uint_32 y = 0; y < png_handler.height(); ++y) {
+ png_read_row(png_handler.png_ptr(), row.data(), nullptr);
int h = (row[3] << 8) | row[2];
std::string loc(reinterpret_cast<char*>(&row[5]));
if (!loc.empty()) {
result.push_back(loc);
}
for (int i = 0; i < h; ++i, ++y) {
- png_read_row(png_ptr, row.data(), NULL);
+ png_read_row(png_handler.png_ptr(), row.data(), nullptr);
}
}
- png_destroy_read_struct(&png_ptr, &info_ptr, nullptr);
return result;
}
int res_create_localized_alpha_surface(const char* name,
const char* locale,
GRSurface** pSurface) {
- GRSurface* surface = NULL;
- int result = 0;
- png_structp png_ptr = NULL;
- png_infop info_ptr = NULL;
- png_uint_32 width, height;
- png_byte channels;
- png_uint_32 y;
- std::vector<unsigned char> row;
+ *pSurface = nullptr;
+ if (locale == nullptr) {
+ return 0;
+ }
- *pSurface = NULL;
+ PngHandler png_handler(name);
+ if (!png_handler) return png_handler.error_code();
- if (locale == NULL) {
- return result;
+ if (png_handler.channels() != 1) {
+ return -7;
+ }
+
+ png_structp png_ptr = png_handler.png_ptr();
+ png_uint_32 width = png_handler.width();
+ png_uint_32 height = png_handler.height();
+
+ for (png_uint_32 y = 0; y < height; ++y) {
+ std::vector<unsigned char> row(width);
+ png_read_row(png_ptr, row.data(), nullptr);
+ int w = (row[1] << 8) | row[0];
+ int h = (row[3] << 8) | row[2];
+ __unused int len = row[4];
+ char* loc = reinterpret_cast<char*>(&row[5]);
+
+ if (y + 1 + h >= height || matches_locale(loc, locale)) {
+ printf(" %20s: %s (%d x %d @ %d)\n", name, loc, w, h, y);
+
+ GRSurface* surface = malloc_surface(w * h);
+ if (!surface) {
+ return -8;
+ }
+ surface->width = w;
+ surface->height = h;
+ surface->row_bytes = w;
+ surface->pixel_bytes = 1;
+
+ for (int i = 0; i < h; ++i, ++y) {
+ png_read_row(png_ptr, row.data(), nullptr);
+ memcpy(surface->data + i * w, row.data(), w);
+ }
+
+ *pSurface = surface;
+ break;
}
- result = open_png(name, &png_ptr, &info_ptr, &width, &height, &channels);
- if (result < 0) return result;
-
- if (channels != 1) {
- result = -7;
- goto exit;
+ for (int i = 0; i < h; ++i, ++y) {
+ png_read_row(png_ptr, row.data(), nullptr);
}
+ }
- row.resize(width);
- for (y = 0; y < height; ++y) {
- png_read_row(png_ptr, row.data(), NULL);
- int w = (row[1] << 8) | row[0];
- int h = (row[3] << 8) | row[2];
- __unused int len = row[4];
- char* loc = reinterpret_cast<char*>(&row[5]);
-
- if (y+1+h >= height || matches_locale(loc, locale)) {
- printf(" %20s: %s (%d x %d @ %d)\n", name, loc, w, h, y);
-
- surface = malloc_surface(w*h);
- if (surface == NULL) {
- result = -8;
- goto exit;
- }
- surface->width = w;
- surface->height = h;
- surface->row_bytes = w;
- surface->pixel_bytes = 1;
-
- int i;
- for (i = 0; i < h; ++i, ++y) {
- png_read_row(png_ptr, row.data(), NULL);
- memcpy(surface->data + i*w, row.data(), w);
- }
-
- *pSurface = surface;
- break;
- } else {
- int i;
- for (i = 0; i < h; ++i, ++y) {
- png_read_row(png_ptr, row.data(), NULL);
- }
- }
- }
-
-exit:
- png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
- if (result < 0 && surface != NULL) free(surface);
- return result;
+ return 0;
}
void res_free_surface(GRSurface* surface) {
- free(surface);
+ free(surface);
}