Skip to content

Commit

Permalink
dt_image_t not available fixes (proposal shutdown part 3)
Browse files Browse the repository at this point in the history 
If we get `dt_image_t *img = dt_image_cache_get()` from the image cache, img can be NULL
for various reasons.

In all those cases we must not de-reference img in any way and must use a meaningful
value - like when getting the imgid - and avoid further processing on that data.

We had that being checked in most cases, a few ones have been overseen yet.
  • Loading branch information
@jenshannoschwalm
jenshannoschwalm committed Dec 25, 2024
1 parent 2134308 commit 5319fa8
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 26 deletions.
4 changes: 2 additions & 2 deletions src/common/datetime.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ GTimeSpan dt_datetime_now_to_gtimespan(void)
void dt_datetime_exif_to_img(dt_image_t *img,
const char *exif)
{
if(!exif) return;
if(!exif || !img) return;
GDateTime *gdt = dt_datetime_exif_to_gdatetime(exif, darktable.utc_tz);
if(gdt)
{
Expand All @@ -247,7 +247,7 @@ gboolean dt_datetime_img_to_exif(char *exif,
const size_t exif_size,
const dt_image_t *img)
{
return dt_datetime_gtimespan_to_exif(exif, exif_size, img->exif_datetime_taken);
return img ? dt_datetime_gtimespan_to_exif(exif, exif_size, img->exif_datetime_taken) : FALSE;
}

GDateTime *dt_datetime_exif_to_gdatetime(const char *exif,
Expand Down
24 changes: 22 additions & 2 deletions src/common/exif.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2161,6 +2161,11 @@ static bool _exif_decode_exif_data(dt_image_t *img, Exiv2::ExifData &exifData)

void dt_exif_apply_default_metadata(dt_image_t *img)
{
if(!img)
{
dt_print(DT_DEBUG_ALWAYS, "[dt_exif_apply_default_metadata] failed as no img was provided");
return;
}
if(dt_conf_get_bool("ui_last/import_apply_metadata")
&& !(img->job_flags & DT_IMAGE_JOB_NO_METADATA))
{
Expand Down Expand Up @@ -2224,6 +2229,11 @@ gboolean dt_exif_read_from_blob(dt_image_t *img,
uint8_t *blob,
const int size)
{
if(!img)
{
dt_print(DT_DEBUG_ALWAYS, "[exiv2 dt_exif_read_from_blob] failed as no img was provided");
return TRUE;
}
try
{
Exiv2::ExifData exifData;
Expand Down Expand Up @@ -2305,6 +2315,11 @@ gboolean dt_exif_get_thumbnail(const char *path,
gboolean dt_exif_read(dt_image_t *img,
const char *path)
{
if(!img)
{
dt_print(DT_DEBUG_ALWAYS, "[dt_exif_read] failed as no img was provided");
return TRUE;
}
// At least set 'datetime taken' to something useful in case there is
// no Exif data in this file (pfm, png, ...)
struct stat statbuf;
Expand Down Expand Up @@ -2762,7 +2777,7 @@ int dt_exif_read_blob(uint8_t **buf,
// GPS data
_remove_exif_geotag(exifData);
const dt_image_t *cimg = dt_image_cache_get(darktable.image_cache, imgid, 'r');
if(!std::isnan(cimg->geoloc.longitude) && !std::isnan(cimg->geoloc.latitude))
if(cimg && !std::isnan(cimg->geoloc.longitude) && !std::isnan(cimg->geoloc.latitude))
{
exifData["Exif.GPSInfo.GPSVersionID"] = "02 02 00 00";
exifData["Exif.GPSInfo.GPSLongitudeRef"] = (cimg->geoloc.longitude < 0) ? "W" : "E";
Expand All @@ -2783,7 +2798,7 @@ int dt_exif_read_blob(uint8_t **buf,
g_free(long_str);
g_free(lat_str);
}
if(!std::isnan(cimg->geoloc.elevation))
if(cimg && !std::isnan(cimg->geoloc.elevation))
{
exifData["Exif.GPSInfo.GPSVersionID"] = "02 02 00 00";
exifData["Exif.GPSInfo.GPSAltitudeRef"] = (cimg->geoloc.elevation < 0) ? "1" : "0";
Expand Down Expand Up @@ -3785,6 +3800,11 @@ gboolean dt_exif_xmp_read(dt_image_t *img,
const char *filename,
const int history_only)
{
if(!img)
{
dt_print(DT_DEBUG_ALWAYS, "[dt_exif_xmp_read] failed as no img was provided for '%s'", filename);
return TRUE;
}
// Exclude pfm to avoid stupid errors on the console
const char *c = filename + strlen(filename) - 4;
if(c >= filename && !strcmp(c, ".pfm")) return TRUE;
Expand Down
40 changes: 26 additions & 14 deletions src/common/grouping.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void dt_grouping_add_to_group(const dt_imgid_t group_id,
dt_grouping_remove_from_group(image_id);

dt_image_t *img = dt_image_cache_get(darktable.image_cache, image_id, 'w');
if(!img) return;
img->group_id = group_id;
dt_image_cache_write_release_info(darktable.image_cache, img,
DT_IMAGE_CACHE_SAFE, "dt_grouping_add_to_group");
Expand All @@ -65,7 +66,7 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
GList *imgs = NULL;

const dt_image_t *img = dt_image_cache_get(darktable.image_cache, image_id, 'r');
const int img_group_id = img->group_id;
const dt_imgid_t img_group_id = img ? img->group_id : NO_IMGID;
dt_image_cache_read_release(darktable.image_cache, img);
if(img_group_id == image_id)
{
Expand All @@ -84,10 +85,13 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
if(!dt_is_valid_imgid(new_group_id))
new_group_id = other_id;
dt_image_t *other_img = dt_image_cache_get(darktable.image_cache, other_id, 'w');
other_img->group_id = new_group_id;
dt_image_cache_write_release_info(darktable.image_cache, other_img,
if(other_img)
{
other_img->group_id = new_group_id;
dt_image_cache_write_release_info(darktable.image_cache, other_img,
DT_IMAGE_CACHE_SAFE, "dt_grouping_add_to_group");
imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
}
}
sqlite3_finalize(stmt);
if(dt_is_valid_imgid(new_group_id))
Expand Down Expand Up @@ -122,13 +126,15 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
{
// change the group_id for this image.
dt_image_t *wimg = dt_image_cache_get(darktable.image_cache, image_id, 'w');
new_group_id = wimg->group_id;
wimg->group_id = image_id;
dt_image_cache_write_release_info(darktable.image_cache, wimg,
if(wimg)
{
new_group_id = wimg->group_id;
wimg->group_id = image_id;
dt_image_cache_write_release_info(darktable.image_cache, wimg,
DT_IMAGE_CACHE_SAFE, "dt_grouping_add_to_group");
imgs = g_list_prepend(imgs, GINT_TO_POINTER(image_id));
// refresh also the group leader which may be alone now
imgs = g_list_prepend(imgs, GINT_TO_POINTER(img_group_id));
imgs = g_list_prepend(imgs, GINT_TO_POINTER(image_id));
// refresh also the group leader which may be alone now
imgs = g_list_prepend(imgs, GINT_TO_POINTER(img_group_id));
#ifdef USE_LUA
dt_lua_async_call_alien(dt_lua_event_trigger_wrapper,
0, NULL, NULL,
Expand All @@ -138,6 +144,7 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
LUA_ASYNC_TYPENAME, "dt_lua_image_t", GINT_TO_POINTER(img_group_id),
LUA_ASYNC_DONE);
#endif
}
}
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_IMAGE_INFO_CHANGED, imgs);

Expand All @@ -150,8 +157,10 @@ dt_imgid_t dt_grouping_change_representative(const dt_imgid_t image_id)
sqlite3_stmt *stmt;

dt_image_t *img = dt_image_cache_get(darktable.image_cache, image_id, 'r');
const dt_imgid_t group_id = img->group_id;
const dt_imgid_t group_id = img ? img->group_id : NO_IMGID;
dt_image_cache_read_release(darktable.image_cache, img);
if(!dt_is_valid_imgid(group_id))
return group_id;

GList *imgs = NULL;
DT_DEBUG_SQLITE3_PREPARE_V2(dt_database_get(darktable.db), "SELECT id FROM main.images WHERE group_id = ?1", -1,
Expand All @@ -161,11 +170,14 @@ dt_imgid_t dt_grouping_change_representative(const dt_imgid_t image_id)
{
const dt_imgid_t other_id = sqlite3_column_int(stmt, 0);
dt_image_t *other_img = dt_image_cache_get(darktable.image_cache, other_id, 'w');
other_img->group_id = image_id;
dt_image_cache_write_release_info(darktable.image_cache, other_img,
if(other_img)
{
other_img->group_id = image_id;
dt_image_cache_write_release_info(darktable.image_cache, other_img,
DT_IMAGE_CACHE_SAFE,
"dt_grouping_change_representative");
imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
}
}
sqlite3_finalize(stmt);
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_IMAGE_INFO_CHANGED, imgs);
Expand Down
18 changes: 10 additions & 8 deletions src/common/image.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1097,11 +1097,11 @@ void dt_image_set_raw_aspect_ratio(const dt_imgid_t imgid)
image->aspect_ratio = (float )image->p_width / (float )(MAX(1, image->p_height));
else
image->aspect_ratio = (float )image->p_height / (float )(MAX(1, image->p_width));
}
/* store */
dt_image_cache_write_release_info(darktable.image_cache, image,
/* store */
dt_image_cache_write_release_info(darktable.image_cache, image,
DT_IMAGE_CACHE_SAFE,
"dt_image_set_raw_aspect_ratio");
}
}

void dt_image_set_aspect_ratio_to(const dt_imgid_t imgid,
Expand Down Expand Up @@ -1160,17 +1160,19 @@ void dt_image_reset_aspect_ratio(const dt_imgid_t imgid, const gboolean raise)
dt_image_t *image = dt_image_cache_get(darktable.image_cache, imgid, 'w');

/* set image aspect ratio */
if(image) image->aspect_ratio = 0.f;

/* store in db, but don't synch XMP */
dt_image_cache_write_release_info(darktable.image_cache, image,
if(image)
{
image->aspect_ratio = 0.f;
/* store in db, but don't synch XMP */
dt_image_cache_write_release_info(darktable.image_cache, image,
DT_IMAGE_CACHE_RELAXED,
"dt_image_reset_aspect_ratio");

if(image && raise && darktable.collection->params.sorts[DT_COLLECTION_SORT_ASPECT_RATIO])
if(raise && darktable.collection->params.sorts[DT_COLLECTION_SORT_ASPECT_RATIO])
dt_collection_update_query(darktable.collection, DT_COLLECTION_CHANGE_RELOAD,
DT_COLLECTION_PROP_ASPECT_RATIO,
g_list_prepend(NULL, GINT_TO_POINTER(imgid)));
}
}

float dt_image_set_aspect_ratio(const dt_imgid_t imgid, const gboolean raise)
Expand Down
7 changes: 7 additions & 0 deletions src/imageio/imageio_rawspeed.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ dt_imageio_retval_t dt_imageio_open_rawspeed(dt_image_t *img,
if(_ignore_image(filename))
return DT_IMAGEIO_UNSUPPORTED_FORMAT;

if(!img)
{
dt_print(DT_DEBUG_ALWAYS, "[dt_imageio_open_rawspeed] failed to get dt_image_t for '%s' at %p",
filename, mbuf);
return DT_IMAGEIO_LOAD_FAILED;
}

if(!img->exif_inited)
(void)dt_exif_read(img, filename);

Expand Down

0 comments on commit 5319fa8

Please sign in to comment.