Evas/cserve2: Fix valgrind warning about uninitialized memory

In cserve2, a shortcut was taken to check if two images were the
same, using memcmp() on the Evas_Image_Load_Opts struct. But it
seems some empty areas in the struct are uninitialized, potentially
making memcmp() fail when the images were actually the same.

This is a minor issue since this function is called only when
bypassing the socket wait.

Also, memset load_opts to 0 and copy all the fields to avoid
the same warning in socket send().

I'm just wondering about the performance impact vs. memcpy/memcmp.
This commit is contained in:
Jean-Philippe Andre 2014-01-09 16:26:18 +09:00
parent 3b06d11fdf
commit b84787bf57
1 changed files with 72 additions and 12 deletions

View File

@ -942,6 +942,60 @@ _evas_image_load_opts_empty(Evas_Image_Load_Opts *lo)
&& (lo->scale_load.scale_hint == 0)); // Skip smooth
}
// Valgrind complains about uninitialized memory, because the load_opts
// can be allocated on the stack, and each field is set independently,
// leaving empty spaces in the struct non initialized. So, compare fields
// one by one, don't take shortcuts like memcmp.
static Eina_Bool
_evas_image_load_opts_equal(const Evas_Image_Load_Opts *lo1,
const Evas_Image_Load_Opts *lo2)
{
return ((lo1->scale_down_by == lo2->scale_down_by)
&& (lo1->dpi == lo2->dpi)
&& (lo1->w == lo2->w)
&& (lo1->h == lo2->h)
&& (lo1->region.x == lo2->region.x)
&& (lo1->region.y == lo2->region.y)
&& (lo1->region.w == lo2->region.w)
&& (lo1->region.h == lo2->region.h)
&& (lo1->scale_load.src_x == lo2->scale_load.src_x)
&& (lo1->scale_load.src_y == lo2->scale_load.src_y)
&& (lo1->scale_load.src_w == lo2->scale_load.src_w)
&& (lo1->scale_load.src_h == lo2->scale_load.src_h)
&& (lo1->scale_load.dst_w == lo2->scale_load.dst_w)
&& (lo1->scale_load.dst_h == lo2->scale_load.dst_h)
&& (lo1->scale_load.smooth == lo2->scale_load.smooth)
&& (lo1->scale_load.scale_hint == lo2->scale_load.scale_hint)
&& (lo1->orientation == lo2->orientation)
&& (lo1->degree == lo2->degree));
}
static void
_evas_image_load_opts_set(Evas_Image_Load_Opts *lo1,
const Evas_Image_Load_Opts *lo2)
{
memset(lo1, 0, sizeof(Evas_Image_Load_Opts));
lo1->scale_down_by = lo2->scale_down_by;
lo1->dpi = lo2->dpi;
lo1->w = lo2->w;
lo1->h = lo2->h;
lo1->region.x = lo2->region.x;
lo1->region.y = lo2->region.y;
lo1->region.w = lo2->region.w;
lo1->region.h = lo2->region.h;
lo1->scale_load.src_x = lo2->scale_load.src_x;
lo1->scale_load.src_y = lo2->scale_load.src_y;
lo1->scale_load.src_w = lo2->scale_load.src_w;
lo1->scale_load.src_h = lo2->scale_load.src_h;
lo1->scale_load.dst_w = lo2->scale_load.dst_w;
lo1->scale_load.dst_h = lo2->scale_load.dst_h;
lo1->scale_load.smooth = lo2->scale_load.smooth;
lo1->scale_load.scale_hint = lo2->scale_load.scale_hint;
lo1->orientation = lo2->orientation;
lo1->degree = lo2->degree;
}
static void
_file_hkey_get(char *buf, size_t sz, const char *path, const char *key,
Evas_Image_Load_Opts *lo)
@ -981,7 +1035,8 @@ _image_open_server_send(Image_Entry *ie)
File_Entry *fentry;
Data_Entry *dentry;
const char *file, *key;
Evas_Image_Load_Opts *opts = NULL;
Evas_Image_Load_Opts opts;
Eina_Bool has_load_opts = EINA_FALSE;
if (cserve2_init == 0)
{
@ -990,7 +1045,10 @@ _image_open_server_send(Image_Entry *ie)
}
if (!_evas_image_load_opts_empty(&ie->load_opts))
opts = &ie->load_opts;
{
_evas_image_load_opts_set(&opts, &ie->load_opts);
has_load_opts = EINA_TRUE;
}
ie->data1 = NULL;
ie->data2 = NULL;
@ -1011,7 +1069,7 @@ _image_open_server_send(Image_Entry *ie)
hkey_len = flen + klen + 1024;
hkey = alloca(hkey_len);
_file_hkey_get(hkey, hkey_len, filebuf, key, opts);
_file_hkey_get(hkey, hkey_len, filebuf, key, (has_load_opts ? &opts : NULL));
fentry = eina_hash_find(_file_entries, hkey);
if (!fentry)
{
@ -1036,18 +1094,17 @@ _image_open_server_send(Image_Entry *ie)
}
memset(&msg_open, 0, sizeof(msg_open));
msg_open.base.rid = _next_rid();
msg_open.base.type = CSERVE2_OPEN;
msg_open.file_id = fentry->file_id;
msg_open.path_offset = 0;
msg_open.key_offset = flen;
msg_open.has_load_opts = (opts != NULL);
msg_open.has_load_opts = has_load_opts;
msg_open.image_id = ++_data_id;
size = sizeof(msg_open) + flen + klen;
if (opts)
size += sizeof(*opts);
if (has_load_opts)
size += sizeof(opts);
buf = malloc(size);
if (!buf)
{
@ -1059,8 +1116,8 @@ _image_open_server_send(Image_Entry *ie)
memcpy(buf, &msg_open, sizeof(msg_open));
memcpy(buf + sizeof(msg_open), filebuf, flen);
memcpy(buf + sizeof(msg_open) + flen, key, klen);
if (opts)
memcpy(buf + sizeof(msg_open) + flen + klen, opts, sizeof(*opts));
if (has_load_opts)
memcpy(buf + sizeof(msg_open) + flen + klen, &opts, sizeof(opts));
if (!_server_send(buf, size, _image_opened_cb, ie))
{
@ -2798,9 +2855,12 @@ _shared_file_data_get_by_id(unsigned int id)
static inline Eina_Bool
_shared_image_entry_image_data_match(Image_Entry *ie, const Image_Data *id)
{
int cmp;
cmp = memcmp(&ie->load_opts, &id->opts, sizeof(ie->load_opts));
if (!cmp)
const Evas_Image_Load_Opts *lo1, *lo2;
lo1 = &ie->load_opts;
lo2 = &id->opts;
if (_evas_image_load_opts_equal(lo1, lo2))
{
DBG("Found loaded image entry at %d", id->id);
return EINA_TRUE;