[Exif] - fixing memory allocation and time bugs

Change-Id: I60ff6c565325ef8dfc9b3183fa13afbfb5ec0928
Signed-off-by: Andrzej Popowski <a.popowski@samsung.com>

diff --git a/src/exif/exif_tag_saver.cc b/src/exif/exif_tag_saver.cc
index 88c0a3c..cbb6e02 100755 (executable)
--- a/src/exif/exif_tag_saver.cc
+++ b/src/exif/exif_tag_saver.cc
@@ -104,6 +104,11 @@ void ExifTagSaver::saveToExif(const std::string& value, ExifTag tag,
     entry->components = new_len;
 
     entry->data = static_cast<unsigned char*>(malloc(entry->size));
+    if (entry->data == nullptr) {
+      LoggerE("Function malloc returned nullptr");
+      return;
+    }
+
     memcpy(entry->data, value.c_str(), value.length());
     if (add_zero_character) {
       entry->data[value.length()] = '\0';
@@ -130,6 +135,10 @@ void ExifTagSaver::saveToExif(const Rational& value, ExifTag tag,
 
     entry->size = ExifTypeInfo::RationalSize;
     entry->data = static_cast<unsigned char*>(malloc(entry->size));
+    if (entry->data == nullptr) {
+      LoggerE("Function malloc returned nullptr");
+      return;
+    }
     memset(entry->data, 0, entry->size);
   }
 
@@ -163,6 +172,10 @@ void ExifTagSaver::saveToExif(const Rationals& value, ExifTag tag,
 
     entry->size = required_size;
     entry->data = static_cast<unsigned char*>(malloc(entry->size));
+    if (entry->data == nullptr) {
+      LoggerE("Function malloc returned nullptr");
+      return;
+    }
     memset(entry->data, 0, entry->size);
   }
 
@@ -196,7 +209,7 @@ void ExifTagSaver::saveToExif(std::vector<long long int>& value,
     case EXIF_FORMAT_SLONG:
       break;
     default:
-      LoggerE("output ExifFormat: %d is not supported!");
+      LoggerE("output ExifFormat: %d is not supported!", store_as);
       return;
   }
   entry->format = store_as;
@@ -211,6 +224,10 @@ void ExifTagSaver::saveToExif(std::vector<long long int>& value,
 
     entry->size = required_size;
     entry->data = static_cast<unsigned char*>(malloc(entry->size));
+    if (entry->data == nullptr) {
+      LoggerE("Function malloc returned nullptr");
+      return;
+    }
     memset(entry->data, 0, entry->size);
   }
   entry->components = num_elements;
@@ -296,13 +313,25 @@ ExifEntry* ExifTagSaver::prepareEntry(ExifData* exif_data, ExifTag tag) {
 
   ExifEntry* exif_entry = exif_data_get_entry(exif_data, tag);
   if (!exif_entry) {
-    exif_entry = createNewTag(exif_data, deduceIfdSection(tag),
-        deduceDataFormat(tag), tag);
+    ExifIfd exif_ifd;
+    common::PlatformResult ret = deduceIfdSection(tag, &exif_ifd);
+    if (!ret) {
+      LoggerE("Couldn't deduce ifd section: %s", ret.message().c_str());
+      return nullptr;
+    }
+
+    ExifFormat exif_format;
+    ret = deduceDataFormat(tag, &exif_format);
+    if (!ret) {
+      LoggerE("Couldn't deduce data format: %s", ret.message().c_str());
+      return nullptr;
+    }
+    exif_entry = createNewTag(exif_data, exif_ifd, exif_format, tag);
   }
 
   if (!exif_entry) {
     LoggerE("Couldn't create new Exif tag");
-    return NULL;
+    return nullptr;
   }
 
   exif_entry_initialize(exif_entry, tag);
@@ -315,14 +344,18 @@ ExifEntry* ExifTagSaver::createNewTag(ExifData* exif_data, ExifIfd ifd,
   LoggerD("Creating new tag: %d", tag);
 
   ExifEntry* new_entry = exif_entry_new();
-  new_entry->tag = tag;
-  new_entry->format = format;
-  exif_content_add_entry(exif_data->ifd[ifd], new_entry);
-  exif_entry_initialize(new_entry, tag);
+  if (new_entry == nullptr) {
+    LoggerE("Function exif_entry_new returned nullptr");
+  } else {
+    new_entry->tag = tag;
+    new_entry->format = format;
+    exif_content_add_entry(exif_data->ifd[ifd], new_entry);
+    exif_entry_initialize(new_entry, tag);
+  }
   return new_entry;
 }
 
-ExifIfd ExifTagSaver::deduceIfdSection(ExifTag tag) {
+common::PlatformResult ExifTagSaver::deduceIfdSection(ExifTag tag, ExifIfd* exif_ifd) {
   LoggerD("Entered");
   // TODO EXIF_TAG_* and EXIF_TAG_GPS_* are sharing same values,
   // they shouldn't be used in one switch statement.
@@ -334,7 +367,8 @@ ExifIfd ExifTagSaver::deduceIfdSection(ExifTag tag) {
     case EXIF_TAG_IMAGE_WIDTH:
     case EXIF_TAG_IMAGE_LENGTH:
     case EXIF_TAG_ORIENTATION:
-      return EXIF_IFD_0;
+      *exif_ifd = EXIF_IFD_0;
+      break;
 
     // Tags in IFD_EXIF Section
     case EXIF_TAG_USER_COMMENT:
@@ -346,7 +380,8 @@ ExifIfd ExifTagSaver::deduceIfdSection(ExifTag tag) {
     case EXIF_TAG_WHITE_BALANCE:
     case EXIF_TAG_FLASH:
     case EXIF_TAG_FOCAL_LENGTH:
-      return EXIF_IFD_EXIF;
+      *exif_ifd = EXIF_IFD_EXIF;
+      break;
 
     // Tags in IFD_GPS Section
     case EXIF_TAG_GPS_LATITUDE_REF:
@@ -358,16 +393,19 @@ ExifIfd ExifTagSaver::deduceIfdSection(ExifTag tag) {
     case EXIF_TAG_GPS_TIME_STAMP:
     case EXIF_TAG_GPS_PROCESSING_METHOD:
     case EXIF_TAG_GPS_DATE_STAMP:
-      return EXIF_IFD_GPS;
+      *exif_ifd = EXIF_IFD_GPS;
+      break;
 
     // Tags in other sections
     default:
       LoggerE("Unsupported tag: %d", tag);
-      // TODO handle error
+      return common::PlatformResult(common::ErrorCode::UNKNOWN_ERR, "Unsupported tag");
   }
+
+  return common::PlatformResult(common::ErrorCode::NO_ERROR);
 }
 
-ExifFormat ExifTagSaver::deduceDataFormat(ExifTag tag) {
+common::PlatformResult ExifTagSaver::deduceDataFormat(ExifTag tag, ExifFormat* exif_format) {
   LoggerD("Entered");
   // TODO EXIF_TAG_* and EXIF_TAG_GPS_* are sharing same values,
   // they shouldn't be used in one switch statement.
@@ -375,19 +413,22 @@ ExifFormat ExifTagSaver::deduceDataFormat(ExifTag tag) {
   switch (static_cast<unsigned int>(tag)) {
     // Tags with byte type:
     case EXIF_TAG_GPS_ALTITUDE_REF:
-      return EXIF_FORMAT_BYTE;
+      *exif_format = EXIF_FORMAT_BYTE;
+      break;
 
     // Tags with long type:
     case EXIF_TAG_IMAGE_WIDTH:
     case EXIF_TAG_IMAGE_LENGTH:
-      return EXIF_FORMAT_LONG;
+      *exif_format = EXIF_FORMAT_LONG;
+      break;
 
     // Tags with short type:
     case EXIF_TAG_ORIENTATION:
     case EXIF_TAG_EXPOSURE_PROGRAM:
     case EXIF_TAG_WHITE_BALANCE:
     case EXIF_TAG_FLASH:
-      return EXIF_FORMAT_SHORT;
+      *exif_format = EXIF_FORMAT_SHORT;
+      break;
 
     // Tags with ASCII type:
     case EXIF_TAG_MAKE:
@@ -396,7 +437,8 @@ ExifFormat ExifTagSaver::deduceDataFormat(ExifTag tag) {
     case EXIF_TAG_GPS_LATITUDE_REF:
     case EXIF_TAG_GPS_LONGITUDE_REF:
     case EXIF_TAG_GPS_DATE_STAMP:
-      return EXIF_FORMAT_ASCII;
+      *exif_format = EXIF_FORMAT_ASCII;
+      break;
 
     // Tags with rational type:
     case EXIF_TAG_EXPOSURE_TIME:
@@ -407,18 +449,22 @@ ExifFormat ExifTagSaver::deduceDataFormat(ExifTag tag) {
     case EXIF_TAG_GPS_ALTITUDE:
     case EXIF_TAG_GPS_TIME_STAMP:
     case EXIF_TAG_ISO_SPEED_RATINGS:
-      return EXIF_FORMAT_RATIONAL;
+      *exif_format = EXIF_FORMAT_RATIONAL;
+      break;
 
     // Tags with undefined type:
     case EXIF_TAG_USER_COMMENT:
     case EXIF_TAG_GPS_PROCESSING_METHOD:
-      return EXIF_FORMAT_UNDEFINED;
+      *exif_format = EXIF_FORMAT_UNDEFINED;
+      break;
 
     // Unsupported tags:
     default:
       LoggerE("Unsupported tag: %d", tag);
-      // TODO handle error
+      return common::PlatformResult(common::ErrorCode::UNKNOWN_ERR, "Unsupported tag");
   }
+
+  return common::PlatformResult(common::ErrorCode::NO_ERROR);
 }
 
 }  // namespace exif

diff --git a/src/exif/exif_tag_saver.h b/src/exif/exif_tag_saver.h
index 8c4c56e..1120fd5 100755 (executable)
--- a/src/exif/exif_tag_saver.h
+++ b/src/exif/exif_tag_saver.h
@@ -22,6 +22,7 @@
 #include <string>
 #include <vector>
 
+#include "common/platform_result.h"
 #include "exif_gps_location.h"
 
 namespace extension {
@@ -46,8 +47,8 @@ class ExifTagSaver {
 
  private:
   static ExifEntry* prepareEntry(ExifData* exif_data, ExifTag tag);
-  static ExifIfd deduceIfdSection(ExifTag tag);
-  static ExifFormat deduceDataFormat(ExifTag tag);
+  static common::PlatformResult deduceIfdSection(ExifTag tag, ExifIfd* exif_ifd);
+  static common::PlatformResult deduceDataFormat(ExifTag tag, ExifFormat* exif_format);
   static ExifEntry* createNewTag(ExifData* exif_data, ExifIfd ifd,
       ExifFormat format, ExifTag tag);
 };

diff --git a/src/exif/exif_util.cc b/src/exif/exif_util.cc
index bc23c60..69677a8 100755 (executable)
--- a/src/exif/exif_util.cc
+++ b/src/exif/exif_util.cc
@@ -379,31 +379,34 @@ void ExifUtil::printExifEntryInfo(ExifEntry* entry, ExifData* exif_data) {
 void ExifUtil::extractFromTimeT(const time_t time,
                                 int& out_year, int& out_month, int& out_day,
                                 int& out_hour, int& out_min, int& out_sec) {
-  struct tm* utc = gmtime(&time);
-
-  out_year = utc->tm_year + 1900;
-  out_month = utc->tm_mon + 1;
-  out_day = utc->tm_mday;
-  out_hour = utc->tm_hour;
-  out_min = utc->tm_min;
-  out_sec = utc->tm_sec;
+  struct tm utc;
+  gmtime_r(&time, &utc);
+
+  out_year = utc.tm_year + 1900;
+  out_month = utc.tm_mon + 1;
+  out_day = utc.tm_mday;
+  out_hour = utc.tm_hour;
+  out_min = utc.tm_min;
+  out_sec = utc.tm_sec;
 }
 
 time_t ExifUtil::convertToTimeT(int year, int month, int day,
       int hour, int min, int sec) {
+  struct tm timeinfo = { 0 };
   time_t tmp_time = 0;
-  struct tm* timeinfo = localtime(&tmp_time);
-  timeinfo->tm_year = year - 1900;
-  timeinfo->tm_mon = month - 1;
-  timeinfo->tm_mday = day;
+  localtime_r(&tmp_time, &timeinfo);
 
-  timeinfo->tm_hour = hour;
-  timeinfo->tm_min = min;
-  timeinfo->tm_sec = sec;
+  timeinfo.tm_year = year - 1900;
+  timeinfo.tm_mon = month - 1;
+  timeinfo.tm_mday = day;
+
+  timeinfo.tm_hour = hour;
+  timeinfo.tm_min = min;
+  timeinfo.tm_sec = sec;
 
   // From mktime documentation:
   // "The values of the members tm_wday and tm_yday of timeptr are ignored"
-  return timegm(timeinfo);
+  return timegm(&timeinfo);
 }
 
 }  // namespace exif