Skip to content

Commit

Permalink
dt_image_t not available fixes (proposal shutdown part 5)
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 f2eb1bb commit 8e00c68
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 22 deletions.
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

0 comments on commit 8e00c68

Please sign in to comment.