When prj->base_path is relative, construct path using prj->file_name so base path is relative to the project file directory.
Fixes #698 You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/755
-- Commit Summary --
* geanyctags: Use base path as relative to the project file path
-- File Changes --
M geanyctags/src/geanyctags.c (28)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/755.patch https://github.com/geany/geany-plugins/pull/755.diff
b4n requested changes on this pull request.
Leaks should be fixed.
@@ -205,6 +205,26 @@ static gchar *generate_find_string(GeanyProject *prj)
}
+static const gchar *get_base_path(void) +{ + static gchar *ret = NULL; + GeanyProject *prj = geany_data->app->project; + gchar *project_dir_utf8; + + if (!prj) + return NULL; + + if (g_path_is_absolute(prj->base_path)) + return prj->base_path; + + g_free(ret);
not useful as it's always `NULL` at this point
@@ -205,6 +205,26 @@ static gchar *generate_find_string(GeanyProject *prj)
}
+static const gchar *get_base_path(void)
this function has a leaky semantic: it returns *either* `prj->base_path` *or* an allocated buffer. The caller must then free conditionally, which is tricky at best.
@@ -236,7 +256,7 @@ on_generate_tags(GtkMenuItem *menuitem, gpointer user_data)
tag_filename, """, NULL); #endif
- spawn_cmd(cmd, prj->base_path); + spawn_cmd(cmd, get_base_path());
leaks when `prj->base_path` is relative
@@ -421,7 +441,7 @@ static void find_tags(const gchar *name, gboolean declaration, gboolean case_sen
return;
msgwin_clear_tab(MSG_MESSAGE); - msgwin_set_messages_dir(prj->base_path); + msgwin_set_messages_dir(get_base_path());
same
@@ -445,7 +465,7 @@ static void find_tags(const gchar *name, gboolean declaration, gboolean case_sen
if (!filter_tag(&entry, name_pat, declaration, case_sensitive)) { - path = g_build_filename(prj->base_path, entry.file, NULL); + path = g_build_filename(get_base_path(), entry.file, NULL);
more
@@ -456,7 +476,7 @@ static void find_tags(const gchar *name, gboolean declaration, gboolean case_sen
if (!filter_tag(&entry, name_pat, declaration, case_sensitive)) { if (!path) - path = g_build_filename(prj->base_path, entry.file, NULL); + path = g_build_filename(get_base_path(), entry.file, NULL);
and… it's me again :grin:
b4n approved this pull request.
LGBI
@@ -205,6 +205,26 @@ static gchar *generate_find_string(GeanyProject *prj)
}
+static const gchar *get_base_path(void) +{ + static gchar *ret = NULL;
I'm not particularly fond of the non-reentrant semantic, but it indeed has the advantage of making other code changes simple.
techee commented on this pull request.
@@ -205,6 +205,26 @@ static gchar *generate_find_string(GeanyProject *prj)
}
+static const gchar *get_base_path(void) +{ + static gchar *ret = NULL;
Things start making much more sense once you notice the magic word "static", don't they ;-). And yes, it probably won't win any competition in code beautifulness but despite that it makes other code simpler and easier to maintain.
codebrainz commented on this pull request.
@@ -205,6 +205,26 @@ static gchar *generate_find_string(GeanyProject *prj)
}
+static const gchar *get_base_path(void) +{ + static gchar *ret = NULL;
You could always just return a non-static allocated pointer and free it where used like most other C functions. All it costs is not embedding function calls directly into argument list of other function calls, which is itself a minefield with C's undefined order of argument evaluation.
More importantly though, using `static` in `dlopen`'d modules is particularly a bad idea, not only because of the real leak upon unloading/reloading the plugin, but also because static variables may not be properly reinitialized on subsequent re-loading of the plugin (depends on `RTLD_NODELETE` and likely other flags), so this could result in some kinds of weird double-frees or similar memory corruption, in theory. It's just asking for trouble, IMO.
techee commented on this pull request.
@@ -205,6 +205,26 @@ static gchar *generate_find_string(GeanyProject *prj)
}
+static const gchar *get_base_path(void) +{ + static gchar *ret = NULL;
@codebrainz Thanks. While I wouldn't care much for a standalone application (such code is used in Geany's tag manager for instance), your argument about dangers of `static` in dynamically loaded libraries is something worth considering so I have rewritten the patch to avoid this problem.
b4n approved this pull request.
LGBI
Merged #755.
github-comments@lists.geany.org