@techee commented on this pull request.


In src/tagmanager/tm_ctags.c:

> @@ -241,6 +241,29 @@ void tm_ctags_clear_ignore_symbols(void)
 }
 
 
+static void replace_str(gchar **where, const gchar *what, guint what_len,
+	const gchar *replacement, guint replacement_len)
+{
+	if (where && *where)
+	{
+		guint where_len = strlen(*where);
+		gchar *pos = strstr(*where, what);
+
+		if (pos)
+		{
+			gchar *str = g_malloc(where_len + replacement_len);

It was really just a poor man's "improvement" of the original code which used the hard-coded value 50 (which is OK in this case because the randomized anonymous names generated by ctags are smaller). Since I moved the code to the (wannabe) generic replace_str function, I wanted to get rid of the hard-coded value but was lazy to do it right.


In src/tagmanager/tm_ctags.c:

> @@ -241,6 +241,29 @@ void tm_ctags_clear_ignore_symbols(void)
 }
 
 
+static void replace_str(gchar **where, const gchar *what, guint what_len,
+	const gchar *replacement, guint replacement_len)
+{
+	if (where && *where)
+	{
+		guint where_len = strlen(*where);

OK.


In src/tagmanager/tm_ctags.c:

> +static void replace_str(gchar **where, const gchar *what, guint what_len,
+	const gchar *replacement, guint replacement_len)

Don't worry, I'm not going to use it as a base for some general-purpose string utility library ;-)


In src/tagmanager/tm_ctags.c:

>  				/* We found the parent name in the nested tag scope - replace it
 				 * with the new name. Note: anonymous tag names generated by
 				 * ctags are unique enough that we don't have to check for
 				 * scope separators here. */
-				if (pos)
-				{
-					gchar *str = g_malloc(nested_scope_len + 50);
-					guint prefix_len = pos - nested_tag->scope;
-
-					strncpy(str, nested_tag->scope, prefix_len);
-					strcpy(str + prefix_len, new_name);
-					strcpy(str + prefix_len + new_name_len, pos + orig_name_len);
-					g_free(nested_tag->scope);
-					nested_tag->scope = str;
-				}
+				replace_str(&nested_tag->scope, orig_name, orig_name_len, new_name, new_name_len);
+
+				/* Do the same for var_type as well */
+				replace_str(&nested_tag->var_type, orig_name, orig_name_len, new_name, new_name_len);

Right, I missed we are doing the same replacement once more below. We could maybe even use the GString version, I'm not sure if the performance is too critical here.

I checked what you propose and it looks good to me and unifies the code a little.


In src/tagmanager/tm_ctags.c:

> @@ -241,6 +241,29 @@ void tm_ctags_clear_ignore_symbols(void)
 }
 
 
+static void replace_str(gchar **where, const gchar *what, guint what_len,
+	const gchar *replacement, guint replacement_len)
+{
+	if (where && *where)
+	{
+		guint where_len = strlen(*where);
+		gchar *pos = strstr(*where, what);
+
+		if (pos)
+		{
+			gchar *str = g_malloc(where_len + replacement_len);
+			guint prefix_len = pos - *where;

OK.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <geany/geany/pull/3785/review/2018216104@github.com>