1. It's certainly not the best policy.
I'm a little unclear how this should be changed,
I'll probably shift is around so the test is solely before the parent->type is 
        if ( ! stack->data )

I'm guessing the only way to trigger the current fault would be to craft a 
specific invalid .vik file of Layers within wrong Layers.
This of course would be useful in proving that the fix is right.

2. Yes. This can be clearly improved.
I think simply gl can be reset is just gl = g_list_first ( gl );
Rather than as you say calculating the full list again.

Thanks for spotting these.

Be Seeing You - Rob.
If at first you don't succeed,
then skydiving isn't for you.

From: Kamil Ignacak <kamil.igna...@gmail.com>
Sent: 07 July 2018 13:10:37
To: viking-devel
Subject: [Viking-devel] Two questions: file.c and vikaggregatelayer.c


During my work with code I have found two places in the code that I would like 
to ask about - can you please take a look? The code is from repo's head:

1. file.c:324
int parent_type = VIK_LAYER(stack->data)->type;
if ( ( ! stack->data ) || ((parent_type != VIK_LAYER_AGGREGATE) && (parent_type 

In second line we test if stack->data is NULL, but in first line we dereference 
the stack->data pointer, which may be NULL. I'm no expert on gobjects, but I 
suspect that this dereferencing of potentially NULL pointer (before testing it) 
may be risky.

2. vikaggregatelayer.c/aggregate_layer_search_date():557
if ( !found ) {
   // Reset and try on Waypoints
   gl = NULL;
   gl = vik_aggregate_layer_get_all_layers_of_type ( val, gl, VIK_LAYER_TRW, 

Is the "reset" of gl and a second execution of 
vik_aggregate_layer_get_all_layers_of_type() (which may be time consuming) 
necessary? Would it be possible to save some time and re-use gl (or previously 
stored copy of gl)?

Best regards,

Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Viking-devel mailing list
Viking home page: http://viking.sf.net/

Reply via email to