[Viking-devel] Confusion about convert_dms_to_dec()
Hello, I've been trying to write unit tests for convert_dms_to_dec() function. Judging by the name, and by quickly glancing at implementation, the function should convert coordinate in DMS format into a double, so: "120.20.30" -> some value It would also seem that gdouble d, m, s variables would contain 120, 20 and 30 respectively. m and s are then (at the bottom of the function) divided by 60 (minutes per degree) and 3600 (seconds per degree) and summed to form proper value of degrees in DEC format. However the function doesn't really work this way. g_strtod() used to convert DMS string to doubles first consumes 120.20, and then remaining .30, so the d and m values are set incorrectly, and s is not set at all. This means that division (by 60 and 3600) and summation at the bottom of the function *would* yield bad results. I'm writing "would" because the function is never actually used to convert string with coordinate in DMS format into double. If I see this correctly, it it is always called to convert string in form "xx.yy" (with possible -/w/W/s/S prefix) into double, so these usages will work correctly: g_strtod() will be called only once, and only "gdouble d" will be set. There is only a risk that someone someday will use the function for conversion from DMS to DEC, and the conversion will fail. Also for the way the function is currently used, its implementation is overly complicated :) Please let me know if any of this makes sense. Best regards, Kamil ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Calculating UTM coordinate from screen position
Hello, I would like to ask a question regarding calculation of UTM letter in vikviewport.c/vik_viewport_screen_to_coord() function. The following code is recognizing a possibility that input screen position passed as function's argument may be located in neighbouring UTM zone, therefore a zone_delta is calculated and the zone itself is adjusted (together in easting in that zone). However the code doesn't seem to consider a case where the input screen position may be located in another latitude band. The band letter in resulting utm variable is always set to be the same as band letter of viewport's center coordinate. I think that the letter of resulting utm should be calculated in some way. utm->zone = vvp->center.utm_zone; utm->letter = vvp->center.utm_letter; utm->easting = ( ( x - ( vvp->width_2) ) * vvp->xmpp ) + vvp->center.east_west; zone_delta = floor( (utm->easting - EASTING_OFFSET ) / vvp->utm_zone_width + 0.5 ); utm->zone += zone_delta; utm->easting -= zone_delta * vvp->utm_zone_width; utm->northing = ( ( ( vvp->height_2) - y ) * vvp->ympp ) + vvp->center.north_south; Please let me know your opinion. Best regards, Kamil ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Minor issue with using of union members
Hello, I have noticed few minor inconsistencies in source code of Viking that I would like to report here. 1. vikgpslayer.c/gps_layer_get_param() Assignment of boolean to integer field of an union: case PARAM_REALTIME_UPDATE_STATUSBAR: rv.u = vgl->realtime_update_statusbar; break; In gps_layer_set_param() the value of the parameter is (correctly) taken from boolean field of the union: case PARAM_REALTIME_UPDATE_STATUSBAR: vgl->realtime_update_statusbar = vlsp->data.b; break; Since these are fields of an union it shouldn't really matter. 2. vikmapslayer.c/maps_layer_set_param() Similar issue as above, assignment from integer field of union instead of boolean (for two parameters): case PARAM_AUTODOWNLOAD: vml->autodownload = vlsp->data.b; break; case PARAM_ONLYMISSING: vml->adl_only_missing = vlsp->data.b; break; Best regards, Kamil ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] x/y zoom in Map Layer
Hello, I would like to ask about usage of 'xmapzoom' field of struct _VikMapsLayer in specific context. vikmapslayer.c/maps_layer_draw_section() function is calculating xzoom/yzoom variables like this (line ~1288): xzoom = vml->xmapzoom; yzoom = vml->xmapzoom; which is a bit strange because other places don't use vml->xmapzoom for calculating both x and y parameters. Is it correct to use vml->xmapzoom in both lines? Best regards, Kamil ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
Re: [Viking-devel] Two questions about indices in DEM layer code
> Agreed (I think you mean '0' is a valid array index). Yes, I meant '0' :) > Note this is not quite 'drawing DEM in UTM mode' OK, thank you for clarifying it. > basically some fairly obscure USGS Topo files, that I doubt hardly anyone uses with viking. What does it mean for the future support of the format in Viking? Best regards, Kamil On Sat, Jan 18, 2020 at 7:51 PM Robert Norris wrote: > > > 1. vikdemlayer.c:620 > > > >Here the code makes sure that array index is not out of bounds, but it > seems that the condition should be 'if(new < 0)' rather than 'if(new < 1)'. > '1' is a valid array index. > > Agreed (I think you mean '0' is a valid array index). > > Thus I also think instead of using > prevcolumn = g_ptr_array_index ( dem->columns, x+1); > explicitly use 0, so its: > prevcolumn = g_ptr_array_index ( dem->columns, 0 ); > > Similarly the upper bound should be explicit, so in a few lines further > down, instead of: > nextcolumn = g_ptr_array_index ( dem->columns, x-1); > it should be: > nextcolumn = g_ptr_array_index ( dem->columns, dem->n_columns-1); > > Note that having tried this (as in fixing it to use column 0) there > appears to be little difference in the end results. > > Further note that this is only for the DEM gradient drawing in getting a > colour by checking the change in elevation values. > Currently the code suffers from a small edge effect for drawing each DEM > file, as in working out the colour it only uses the current DEM file; as > opposed to attempting to get values from the neighbouring DEMs (i.e. the > elev at a location outside this DEM) to calculate the gradient across the > boundary. > > Depending on how simple this is to fix or if computationally significant, > I'll at least put a comment in the code about it. > > >2. vikdemlayer.c:787 > > >Here the code draws DEM in UTM mode and is again checking column index, > and I think that it again is making a mistake. The 'x > 0' condition > probably should be replaced with 'x >= 0', because '0' is still a valid > array index. > > Agreed it should be 'x >= 0'. > > Note this is not quite 'drawing DEM in UTM mode', this is for drawing when > the DEM data files themselves are in UTM format (as opposed to setting > Viking to draw in UTM mode). However such DEM files are only available if > you have configured viking compiling with '--enable-dem24k' and then using > a separate program 'sdts2dem' - basically some fairly obscure USGS Topo > files, that I doubt hardly anyone uses with viking. > > Indeed having never tried it before, I run the perl script (that Viking > would invoke) to acquire such DEM files but the website it accesses: " > geocomm.com" doesn't exist anymore. > > > As ever, thank you for taking the time to analyze and commit on the Viking > code. > ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Two questions about indices in DEM layer code
Hello,
I would like to ask about two places in DEM layer code that I think may be
invalid.
1. vikdemlayer.c:620
Here the code makes sure that array index is not out of bounds, but it
seems that the condition should be 'if(new < 0)' rather than 'if(new < 1)'.
'1' is a valid array index.
// get previous and next column. catch out-of-bound.
gint32 new_x = x;
new_x -= gradient_skip_factor;
if(new_x < 1)
prevcolumn = g_ptr_array_index ( dem->columns, x+1);
else
prevcolumn = g_ptr_array_index ( dem->columns, new_x);
2. vikdemlayer.c:787
Here the code draws DEM in UTM mode and is again checking column index, and
I think that it again is making a mistake. The 'x > 0' condition probably
should be replaced with 'x >= 0', because '0' is still a valid array index.
for ( x=start_x, counter.easting = start_eas; [...] ) {
if ( x > 0 && x < dem->n_columns ) {
column = g_ptr_array_index ( dem->columns, x );
Please let me know what you think.
Best regards,
Kamil
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Possible memory leak in Mapnik layer
Hello,
I would like to ask you about potential memory leak in Mapnik layer
code. Here is the suspected place of memory leak:
mapnik_interface.c:247
if ( image.painted() ) {
unsigned char *ImageRawDataPtr = (unsigned char *)
g_malloc(width * 4 * height);
if (!ImageRawDataPtr)
return NULL;
memcpy(ImageRawDataPtr, image.raw_data(), width * height * 4);
pixbuf = gdk_pixbuf_new_from_data(ImageRawDataPtr,
GDK_COLORSPACE_RGB, TRUE, 8, width, height, width * 4, NULL, NULL);
}
Memory is allocated by g_malloc() and passed to
gdk_pixbuf_new_from_data(). Eight argument to the
gdk_pixbuf_new_from_data() function is a "Function used to free the data
when the pixbuf's reference count drops to zero, or NULL if the data
should not be freed."
Since in the code the argument is NULL, I suspect that the
ImageRawDataPtr buffer will never be freed after pixbuf becomes
unused/not referenced anymore.
Am I understanding the gdk_pixbuf_new_from_data() function and its
'destroy_fn' argument correctly?
https://developer.gnome.org/gdk-pixbuf/stable/gdk-pixbuf-Image-Data-in-Memory.html#gdk-pixbuf-new-from-data
Best regards,
Kamil
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Unnecessary action on toggle in viktrwlayer_analysis.c?
Hello, I've been reviewing code in viktrwlayer_analysis.c and came upon include_invisible_toggled_cb() callback. One thing that I noticed is that the function re-generates a list of tracks and layers on every call, i.e. every time an "include invisible" checkbox is toggled. The contents of the list does not depend on the state of the checkbox, so every time the list is the same. The state of checkbox only influences which elements of the list are processed in later stages. I can imagine that current implementation may be useful if list of tracks changes in the background (new tracks are being removed or added) - user just has to click twice to get a new, current list of tracks. Was this the reason behind re-generating list of tracks and layers on every checkbox toggle? Best regards, Kamil ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] vikwebtoolformat.c - problem with LatLon variable?
Hello, I have encountered a piece of code that potentially is incorrect. Can you please take a look? vikwebtoolformat.c:223 struct LatLon llpt; llpt.lat = 0.0; llpt.lon = 0.0; if ( vc ) vik_coord_to_latlon ( vc, &ll ); gchar spointlat[G_ASCII_DTOSTR_BUF_SIZE]; gchar spointlon[G_ASCII_DTOSTR_BUF_SIZE]; g_ascii_dtostr (spointlat, G_ASCII_DTOSTR_BUF_SIZE, llpt.lat); g_ascii_dtostr (spointlon, G_ASCII_DTOSTR_BUF_SIZE, llpt.lon); This is a single block of code starting with definition of llpt variable. The llpt coordinates are set to 0.0/0.0 (initialization), and then vik_coord_to_latlon() is called. The thing that is strange for me is that the function is called on ll variable, not on llpt variable. Later the values of llpt fields (still set to 0.0/0.0) are used to produce spointlat/spointlon strings. I understand that maybe we do want spointlat/spointlon to contain "0.0"/"0.0" strings, but then the call to vik_coord_to_latlon() is unnecessary. Is there an error in this code or am I seeing this completely wrong? Best regards, Kamil ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] curl_download.c: problem with curl_easy_cleanup()
Hello,
I think that I have discovered a small problem in curl_download.c code, can
you please verify? The problem is around line 220:
if (!handle)
curl_easy_cleanup ( curl );
if (curl_send_headers) {
curl_slist_free_all(curl_send_headers);
curl_send_headers = NULL;
curl_easy_setopt ( curl, CURLOPT_HTTPHEADER , NULL);
}
The code first runs curl_easy_cleanup() on curl handle, and later (if
curl_send_headers is true) the code also executes curl_easy_setopt() on the
previously cleaned-up curl handle.
The documentation for curl_easy_cleanup() (
https://curl.haxx.se/libcurl/c/curl_easy_cleanup.html) states:
"Any use of the handle after this function has been called and have
returned, is illegal. curl_easy_cleanup kills the handle and all memory
associated with it!"
curl_easy_cleanup() is also used in two other places. The second place
probably suffers from the same problem.
I didn't verify the third place where the function is called.
curl_easy_cleanup() is inside a wrapper that may be used in less obvious
ways. Perhaps setting the handle to NULL after
curl_easy_cleanup() and then testing if the handle is NULL would help
catching any problems caused by third call to curl_easy_cleanup().
Best regards,
Kamil
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Two issues: copyright notice and g_free()
Hello,
I have spotted two minor issues in viking source code, can you please
verify them?
1. vikwebtoolformat.c/h files have an error in copyright notice: both of
them contain reference to "GNU *Format* Public License". Probably a result
of global search&replace.
2. call to g_free() in line 112 of compression.c file appears to be
unnecessary:
if (GUINT32_FROM_LE(local_file_header->sig) != 0x04034b50) {
...
g_free(unzip_data);
goto end;
}
It's not an error because g_free() can handle NULL pointer argument just
fine.
Best regards,
Kamil
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Comparing coordinates in trw layer code
Hi,
I've been looking at trw code and I noticed one possible problem in code
drawing track. Can you please take a look?
The problem is with this code in viktrwlayer.c (~line 2452):
/*
* If points are the same in display coordinates, don't draw.
*/
if ( x != oldx && y != oldy )
{
vik_viewport_coord_to_screen ( dp->vp, &(tp2->coord), &x, &y );
draw_utm_skip_insignia ( dp->vp, main_gc, x, y );
}
Shouldn't the condition be rather "x != oldx || y != oldy"? Similar block
of code is present few lines above, but the condition contains "||" instead
of "&&":
/*
* If points are the same in display coordinates, don't draw.
*/
if ( x != oldx || y != oldy )
{
if ( draw_track_outline )
vik_viewport_draw_line ( dp->vp, dp->vtl->track_bg_gc, oldx, oldy,
x, y);
else
vik_viewport_draw_line ( dp->vp, main_gc, oldx, oldy, x, y);
}
Best regards,
Kamil
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Possible memory leak through a_dialog_new_track()
Hi!
I think that I have spotted a possible memory leak. Can you please take a
look and verify?
Viking is using a_dialog_new_track() function in two places. In both places
the second argument passed to the function is a freshly allocated string.
The function doesn't deallocate the string explicitly and - if I understand
correctly - the string is not deallocated by gtk widgets used in the
function either.
The code calling a_dialog_new_track() does deallocate a string, but it may
be a different string than the one passed as argument to function:
gchar *name = trw_layer_new_unique_sublayer_name(vtl, ..., ...);
if (a_vik_get_ask_for_create_track_name()) {
/* These two 'name' pointers may have different values. */
name = a_dialog_new_track(vtl, name, FALSE);
if (!name)
return FALSE;
}
new_track_create_common( vtl, name);
g_free(name);
Best regards,
Kamil
--
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-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Two questions: file.c and vikaggregatelayer.c
Hello,
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 != VIK_LAYER_GPS)) )
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, TRUE );
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,
Kamil
--
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-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] a_coords_latlon_to_string() and memoryleak
Hi! I think that I've spotted a small problem with a_coords_latlon_to_string() function in recent viking code. The function returns (through its second and third argument) two freshly allocated strings. In most cases these strings are deallocated properly, but in two cases in vikwindow.c they are not. One such case is in ruler_move(), and another in ruler_click(). Can you please verify and confirm this? Best regards, Kamil -- 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-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] g_get_current_dir() and memoryleak
Hello!
I think that I have spotted a minor memory leak through pointers
returned by g_get_current_dir(). The function is used in several
places in viking source code, but unfortunately in some cases pointer
returned by the function is not freed. One example can be found in
vikgeoreflayer.c:290:
if ( ) {
gchar *cwd = g_get_current_dir();
if ( cwd ) {
rv.s = file_GetRelativeFilename ( cwd, vgl->image );
if ( !rv.s ) rv.s = "";
set = TRUE;
}
}
}
Can you please take a look at usage of the function and confirm that
the problem really exists?
Best regards,
Kamil
--
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-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Four possible problems in viking code
Hello, I think that I've spotted four minor issues in the most recent code of Viking, could you please take a look? 1. globals.c:225: tmp.b = VIK_GPX_EXPORT_WPT_SYM_NAME_TITLECASE; a_preferences_register(&io_prefs[2], tmp, VIKING_PREFERENCES_IO_GROUP_KEY); I think that the assignment should be made to tmp.u instead of tmp.i, because the parameter registered here is of type VIK_LAYER_PARAM_UINT. I'm guessing that current code works just fine because this parameter has only two allowed values (TITLECASE=0 and LOWERCASE=1). 2. globals:329 (related to #1): This line refers to ->b field: val = a_preferences_get(VIKING_PREFERENCES_IO_NAMESPACE "gpx_export_wpt_sym_names")->b; The problem is that the "gpx_export_wpt_sym_names" parameter is of type VIK_LAYER_PARAM_UINT. 3. babel.c:465: return a_babel_convert_from_shellcommand ( vt, process_options->shell_command, process_options->filename, cb, user_data, download_options ); I think that the third function argument in this function call is invalid. Function's prototype states that the argument is 'const char *input_file_type'. Function's body shows that the function expects the argument to hold string suitable for gpsbabel's "-i" command line option. I'm not sure how to verify this in running application, so I may be totally incorrect here. 4. viktrack.c:203: vik_track_set_name(new_tr,tr->name); This is a line from vik_track_copy(), we set a name of new track. The problem is that we already did this in line 182: new_tr->name = g_strdup(tr->name); There is no real error because call to vik_track_set_name() in line 203 doesn't cause memory leak, but (if I see this correctly) line 203 is unnecessary. Best regards, Kamil -- 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-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Two minor memory leaks
Hi! I think that I've found two minor memory leaks in recent viking code. Can you please take a look? 1. The leak happens when a_get_viking_data_home() function is called in modules.c: value returned by this function is not deallocated. 2. The leak happens in waypoint code: destructor function vik_waypoint_free() doesn't deallocate "name" field, even though the field is set using g_strdup() function. Best regards, Kamil -- 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-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Possible memory leak in vikmapslayer.c
Hi!
I think that I have spotted a minor memory leak in Maps Layer code:
In vikmapslayer.c/maps_layer_how_many_maps() (around line 2266) there is
this block of code:
if ( mdi->redownload == REDOWNLOAD_BAD ) {
/* see if this one is bad or what */
GError *gx = NULL;
GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file ( mdi->filename_buf,
&gx );
if (gx || (!pixbuf)) {
mdi->mapstoget++;
}
break;
// Other download cases already considered or just ignored
}
If I understand correctly, gdk_pixbuf_new_from_file() may allocate a new
pixbuf, but it won't be deallocated anywhere. Am I right here?
Moreover if gdk_pixbuf_new_from_file() fails, then gx is set, but is
never deallocated with g_error_free().
Top level comment for maps_layer_how_many_maps() says "Copied from
maps_layer_download_section", so maybe the problems occur somewhere
else, too, but I didn't verify this.
Best regards,
Kamil
--
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-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Memory leak in vikgpslayer.c
Hello, I think that I have found a minor memory leak in vikgpslayer.c, line 1752: vik_trw_layer_add_track(vtl, make_track_name(vtl), vgl->realtime_track); make_track_name() returns freshly malloced buffer that isn't deallocated anywhere. Best regards, Kamil -- 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-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Memory leak in vikmapniklayer.c
Hello,
I think that I've found a small memory leak in vikmapniklayer.c, can you
please take a look?
The leak is in carto_load() function, line 505, the "msg" pointer is
never deallocated:
if ( vw ) {
gchar *msg = g_strdup_printf( "%s: %s", _("Running"), command );
vik_window_statusbar_update( vw, msg, VIK_STATUSBAR_INFO );
vik_window_set_busy_cursor( vw );
}
Best regards,
Kamil
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] viktrwlayer.c: possible memory leak in uniquify functions
Hi! I think that I've found a minor memory leak in viktrwlayer.c file. Can you please take a look? The leak is in two uniquify functions: vik_trw_layer_uniquify_tracks() vik_trw_layer_uniquify_waypoints() Both of the functions allocate a char buffer with new track or waypoint name with trw_layer_new_unique_sublayer_name(), but the buffer is never deallocated. Am I seeing this right? Best regards, Kamil -- ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] Handling of VIK_UNITS_DISTANCE_NAUTICAL_MILES
Hello,
I have noticed that there is a number of places in source code that
don't handle VIK_UNITS_DISTANCE_NAUTICAL_MILES value returned by
a_vik_get_units_distance(). In such places a switch statements handles
KILOMETRES and MILES, but not NAUTICAL_MILES. One such example from
viktrwlayer_tracklist.c:428:
switch ( dist_units ) {
case VIK_UNITS_DISTANCE_MILES:
trk_dist = VIK_METERS_TO_MILES(trk_dist);
break;
default:
trk_dist = trk_dist/1000.0;
break;
}
Other places that I've spotted are:
- viktrwlayer_tpwin.c:519
- viktrwlayer_tracklist.c:625
Is it intentional?
Best regards,
Kamil
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity
planning reports. http://sdm.link/zohodev2dev
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
Re: [Viking-devel] Bug in track statistics code
On 10.08.2016 23:39, Robert Norris wrote: > > I'll try to sort this out when I've had less cider... > I'll just mention that there is similar code in src/viktrwlayer_propwin.cpp/void vik_trw_layer_propwin_run() that shows track statistics. Perhaps it could be somehow reused. -- What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. http://sdm.link/zohodev2dev ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
Re: [Viking-devel] Bug in track statistics code
On 10.08.2016 19:23, Robert Carter wrote:
> Did you try something like?
>
> default:
> tmp_buf = { 0 );
> //VIK_UNITS_SPEED_KILOMETRES_PER_HOUR:
> if ( ts.max_speed > 0 )
> g_snprintf ( tmp_buf, sizeof(tmp_buf), _("%.2f km/h"),
> (double)VIK_MPS_TO_KPH(ts.max_speed) );
> if (tmp_buf[0] != '\0')
> gtk_label_set_text ( GTK_LABEL(content[cnt++]), tmp_buf );
>
I see what you are trying to do, but that's still not good, because now
this code creates a GTK label - or not (there is "content[cnt++]" in the
new 'if'). We would end up with N or N-1 GTK labels.
What we want is something like this:
default:
//VIK_UNITS_SPEED_KILOMETRES_PER_HOUR:
if ( ts.max_speed > 0 )
g_snprintf ( tmp_buf, sizeof(tmp_buf), _("%.2f km/h"),
(double)VIK_MPS_TO_KPH(ts.max_speed) );
else
g_snprintf ( tmp_buf, sizeof(tmp_buf), _("---"));
gtk_label_set_text ( GTK_LABEL(content[cnt++]), tmp_buf );
This is how it's done for other statistics items.
The whole switch statement handling speed statistics item (for different
speed units) should be updated along these lines.
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity
planning reports. http://sdm.link/zohodev2dev
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Bug in track statistics code
Hello,
I think that I found a bug in track statistics code.
Steps to reproduce the problem in viking built from freshly downloaded code:
1. open a gpx file with a single track that has some valid trackpoints;
a TRW layer is created.
2. in the same layer draw a route.
3. in layers panel, mark the track as invisible, keep the route visible.
4. right-click the top layer, from menu select "Statistics".
5. toggle "Include Invisible Items" checkbox at the bottom of the dialog
window.
6. observe values in "Average length" and "Max speed" category - when
the checkbox is cleared, the two values are the same, with the same unit.
I think that the problem can be traced to this piece of code in
src/viktrwlayer_analisis.c, starting in line 314:
default:
//VIK_UNITS_SPEED_KILOMETRES_PER_HOUR:
if ( ts.max_speed > 0 )
g_snprintf ( tmp_buf, sizeof(tmp_buf), _("%.2f km/h"),
(double)VIK_MPS_TO_KPH(ts.max_speed) );
gtk_label_set_text ( GTK_LABEL(content[cnt++]), tmp_buf );
When only visible items are included in statistics (i.e. only the route,
but not the track), ts.max_speed is zero and tmp_buf is not updated
properly, it still contains contents that has been set few lines earlier.
Best regards,
Kamil
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity
planning reports. http://sdm.link/zohodev2dev
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] VikWindow: toggle main menu
Hello,
I have noticed a small problem with toggling main menu visibility. From
vikwindow.c:
static void view_main_menu_cb ( GtkAction *a, VikWindow *vw )
{
[...]
if ( tbutton ) {
gboolean tb_state = gtk_toggle_tool_button_get_active ( tbutton );
if ( next_state != tb_state )
gtk_toggle_tool_button_set_active ( tbutton, next_state );
else
toggle_main_menu ( vw );
}
else
toggle_toolbar ( vw );
}
Calling toggle_toolbar() in this function is incorrect, it should be
toggle_main_menu() instead.
Best regards,
Kamil
--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports. http://pubads.g.doubleclick.net/gampad/clk?id=1444514421&iu=/41014381
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
[Viking-devel] Four more questions about Viking source code
Hello,
While working on a copy of Viking I've found few places in code that I
think may be improved. Can you please take a look?
1.
src/viktrwlayer.c:2193
g_array_index(dp->vtl->track_gc, GdkGC *, 11)
I think that the value '11' is invalid, because in my builds the
following message apears in terminal:
"(viking:3814): Gdk-CRITICAL **: IA__gdk_draw_arc: assertion 'GDK_IS_GC
(gc)' failed"
Since this line is supposed to draw a stop, perhaps
VIK_TRW_LAYER_TRACK_GC_STOP should be used instead.
2.
src/vikviewport.c:1641
vik_coord_load_from_utm ( &test, VIK_VIEWPORT_DRAWMODE_UTM, &u );
Strictly speaking the second argument to the function should be
VIK_COORD_UTM. Both names evaluate to zero, so this works.
3.
src/viktrwlayer.c:3324
GHashTable *vik_trw_layer_get_waypoints_iters ( VikTrwLayer *vtl )
{
return vtl->waypoints;
}
Shouldn't this function return vtl->waypoints_iters?
4.
src/viktrwlayer.c:2178
src/viktrwlayer.c:2365
if ( (!dp->one_zone && !dp->lat_lon) ...
Shouldn't these conditions for drawing a trackpoint and a waypoint be
the same? Currently they are not, second term of each condition is:
((!dp->one_zone) || tp->coord.utm_zone == dp->center->utm_zone)
vs.
( dp->lat_lon || wp->coord.utm_zone == dp->center->utm_zone )
From my understanding (!dp->one_zone) != dp->lat_lon.
Am I mistaken?
Best regards,
Kamil
--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
Re: [Viking-devel] viktrwlayer.cpp: invalid conversion from gpointer {aka void*} to void**
On 09.04.2016 22:00, Kamil Ignacak wrote: > The only other problems that I've noticed are in src/kmz.c: Please disregard the last message, the problem occurred only in my repo. Sorry for spamming the list. Best regards, Kamil -- Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! http://pubads.g.doubleclick.net/ gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532 ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
Re: [Viking-devel] viktrwlayer.cpp: invalid conversion from gpointer {aka void*} to void**
On 09.04.2016 15:28, Robert Norris wrote: > > Please report any more things that might be actual errors, rather than the > C++ compiler being more strict on type conversions issues. > The only other problems that I've noticed are in src/kmz.c: 1. call to g_strinfree() instead of g_string_free() 2. zip_int64_t_t data type instead of zip_int64_t Since I'm done with converting files from *.c to *.cpp, I think that this is the last report of this type. Best regards, Kamil -- Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! http://pubads.g.doubleclick.net/ gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532 ___ Viking-devel mailing list Viking-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/viking-devel Viking home page: http://viking.sf.net/
[Viking-devel] viktrwlayer.cpp: invalid conversion from gpointer {aka void*} to void**
Hi!
I hope that this is a good place to send such information to.
I'm in the process of porting Viking to C++, and during compilation of
src/viktrwlayer.cpp the compiler issued multiple errors about invalid
conversions. Most of them were easy to fix and shouldn't be a problem in
C program, but there are two classes of errors that I think are worth
looking at:
1. Invalid conversion from void* to void**:
viktrwlayer.cpp: In function ‘void waypoint_search_closest_tp(gpointer,
VikWaypoint*, WPSearchParams*)’:
viktrwlayer.cpp:9081:29: error: invalid conversion from ‘gpointer {aka
void*}’ to ‘void**’ [-fpermissive]
params->closest_wp_id = id;
2. Invalid conversion from void* to GtkWidget**
viktrwlayer.cpp:1378:45: error: invalid conversion from ‘gpointer {aka
void*}’ to ‘GtkWidget** {aka _GtkWidget**}’ [-fpermissive]
GtkWidget **ww2 = values[UI_CHG_LABELS];
There are ~45 such errors in viktrwlayer.cpp.
I think that these problems *might* be caused by declarations of some
struct fields and arguments of type "gpointer *", but I may be wrong.
I understand that currently Viking can be compiled and these g++ errors
don't cause any problems in Viking. I just thought that you may be
interested.
I'm not sure if it's ok to post here compilation log +1000 lines long,
so I have just given you two examples. You can reproduce full log for
viktrwlayer.cpp by editing the file name in src/Makefile.am,
regenerating build files and attempting to compile the program.
Best regards,
Kamil
--
___
Viking-devel mailing list
Viking-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/viking-devel
Viking home page: http://viking.sf.net/
