[libcamera-devel] android: jpeg: exif: Use reentrant localtime_r()

9566 diff mbox series

The std::localtime() function isn't thread-safe, and we have no
guarantee whether other threads in the camera service may or may not
call it. Replace it with localtime_r(). This requires switching from
ctime to time.h, as there is no std::localtime_r() function.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/android/jpeg/exif.cpp | 6 ++++--
 src/android/jpeg/exif.h   | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

Hi Laurent,

On 9/10/20 9:44 AM, Laurent Pinchart wrote:
> The std::localtime() function isn't thread-safe, and we have no
> guarantee whether other threads in the camera service may or may not
> call it. Replace it with localtime_r(). This requires switching from
> ctime to time.h, as there is no std::localtime_r() function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <email@uajain.com>
> ---
>   src/android/jpeg/exif.cpp | 6 ++++--
>   src/android/jpeg/exif.h   | 2 +-
>   2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index 1ced55343ee9..c0dbfcc216b9 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -186,9 +186,11 @@ void Exif::setSize(const Size &size)
>   
>   void Exif::setTimestamp(time_t timestamp)
>   {
> +	struct tm tm;
> +	localtime_r(&timestamp, &tm);
> +
>   	char str[20];
> -	std::strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S",
> -		      std::localtime(&timestamp));
> +	strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S", &tm);
>   	std::string ts(str);
>   
>   	setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index 622de4cfd593..f04cefcea74a 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -7,8 +7,8 @@
>   #ifndef __ANDROID_JPEG_EXIF_H__
>   #define __ANDROID_JPEG_EXIF_H__
>   
> -#include <ctime>
>   #include <string>
> +#include <time.h>
>   
>   #include <libexif/exif-data.h>
>

Hi Laurent,

Thanks for your work.

On 2020-09-10 07:14:59 +0300, Laurent Pinchart wrote:
> The std::localtime() function isn't thread-safe, and we have no
> guarantee whether other threads in the camera service may or may not
> call it. Replace it with localtime_r(). This requires switching from
> ctime to time.h, as there is no std::localtime_r() function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/android/jpeg/exif.cpp | 6 ++++--
>  src/android/jpeg/exif.h   | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index 1ced55343ee9..c0dbfcc216b9 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -186,9 +186,11 @@ void Exif::setSize(const Size &size)
>  
>  void Exif::setTimestamp(time_t timestamp)
>  {
> +	struct tm tm;
> +	localtime_r(&timestamp, &tm);
> +
>  	char str[20];
> -	std::strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S",
> -		      std::localtime(&timestamp));
> +	strftime(str, sizeof(str), "%Y:%m:%d %H:%M:%S", &tm);
>  	std::string ts(str);
>  
>  	setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index 622de4cfd593..f04cefcea74a 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -7,8 +7,8 @@
>  #ifndef __ANDROID_JPEG_EXIF_H__
>  #define __ANDROID_JPEG_EXIF_H__
>  
> -#include <ctime>
>  #include <string>
> +#include <time.h>
>  
>  #include <libexif/exif-data.h>
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel