Skip to content
Browse files

Fix crash with bulk pattern replacements (introduced with c83a93e)

"regex_match_text" and "regex_matches" being globals, performing
several searches and then the replacements separately lead to them
having unexpected values, resulting in incorrect behavior and crash.

Fix this by removing the globals and instead make the search functions
return match details.  Not only this fixes the issue, but also make the
code a lot more maintainable by not having globals introducing side
effects (proof of them being an issue is that c83a93e inadvertently
broke things bad).
  • Loading branch information...
1 parent 920969e commit 5412a244ba903624053cdaf7393732bc3af689ea @b4n b4n committed
Showing with 157 additions and 82 deletions.
  1. +15 −11 src/document.c
  2. +3 −1 src/document.h
  3. +116 −66 src/search.c
  4. +23 −4 src/search.h
View
26 src/document.c
@@ -1378,7 +1378,7 @@ static void replace_header_filename(GeanyDocument *doc)
ttf.chrg.cpMax = sci_get_position_from_line(doc->editor->sci, 4);
ttf.lpstrText = filebase;
- if (search_find_text(doc->editor->sci, SCFIND_MATCHCASE | SCFIND_REGEXP, &ttf) != -1)
+ if (search_find_text(doc->editor->sci, SCFIND_MATCHCASE | SCFIND_REGEXP, &ttf, NULL) != -1)
{
sci_set_target_start(doc->editor->sci, ttf.chrgText.cpMin);
sci_set_target_end(doc->editor->sci, ttf.chrgText.cpMax);
@@ -1894,7 +1894,8 @@ gboolean document_search_bar_find(GeanyDocument *doc, const gchar *text, gint fl
* @param original_text Text as it was entered by user, or @c NULL to use @c text
*/
gint document_find_text(GeanyDocument *doc, const gchar *text, const gchar *original_text,
- gint flags, gboolean search_backwards, gboolean scroll, GtkWidget *parent)
+ gint flags, gboolean search_backwards, GeanyMatchInfo **match_,
+ gboolean scroll, GtkWidget *parent)
{
gint selection_end, selection_start, search_pos;
@@ -1921,9 +1922,9 @@ gint document_find_text(GeanyDocument *doc, const gchar *text, const gchar *orig
sci_set_search_anchor(doc->editor->sci);
if (search_backwards)
- search_pos = sci_search_prev(doc->editor->sci, flags, text);
+ search_pos = search_find_prev(doc->editor->sci, text, flags, match_);
else
- search_pos = search_find_next(doc->editor->sci, text, flags);
+ search_pos = search_find_next(doc->editor->sci, text, flags, match_);
if (search_pos != -1)
{
@@ -1954,7 +1955,7 @@ gint document_find_text(GeanyDocument *doc, const gchar *text, const gchar *orig
gint ret;
sci_set_current_position(doc->editor->sci, (search_backwards) ? sci_len : 0, FALSE);
- ret = document_find_text(doc, text, original_text, flags, search_backwards, scroll, parent);
+ ret = document_find_text(doc, text, original_text, flags, search_backwards, match_, scroll, parent);
if (ret == -1)
{ /* return to original cursor position if not found */
sci_set_current_position(doc->editor->sci, selection_start, FALSE);
@@ -1976,6 +1977,7 @@ gint document_replace_text(GeanyDocument *doc, const gchar *find_text, const gch
const gchar *replace_text, gint flags, gboolean search_backwards)
{
gint selection_end, selection_start, search_pos;
+ GeanyMatchInfo *match = NULL;
g_return_val_if_fail(doc != NULL && find_text != NULL && replace_text != NULL, -1);
@@ -1994,7 +1996,7 @@ gint document_replace_text(GeanyDocument *doc, const gchar *find_text, const gch
if (selection_end == selection_start)
{
/* no selection so just find the next match */
- document_find_text(doc, find_text, original_find_text, flags, search_backwards, TRUE, NULL);
+ document_find_text(doc, find_text, original_find_text, flags, search_backwards, NULL, TRUE, NULL);
return -1;
}
/* there's a selection so go to the start before finding to search through it
@@ -2004,20 +2006,22 @@ gint document_replace_text(GeanyDocument *doc, const gchar *find_text, const gch
else
sci_goto_pos(doc->editor->sci, selection_start, TRUE);
- search_pos = document_find_text(doc, find_text, original_find_text, flags, search_backwards, TRUE, NULL);
+ search_pos = document_find_text(doc, find_text, original_find_text, flags, search_backwards, &match, TRUE, NULL);
/* return if the original selected text did not match (at the start of the selection) */
if (search_pos != selection_start)
+ {
+ if (search_pos != -1)
+ geany_match_info_free(match);
return -1;
+ }
if (search_pos != -1)
{
- gint replace_len;
- /* search next/prev will select matching text, which we use to set the replace target */
- sci_target_from_selection(doc->editor->sci);
- replace_len = search_replace_target(doc->editor->sci, replace_text, flags & SCFIND_REGEXP);
+ gint replace_len = search_replace_match(doc->editor->sci, match, replace_text);
/* select the replacement - find text will skip past the selected text */
sci_set_selection_start(doc->editor->sci, search_pos);
sci_set_selection_end(doc->editor->sci, search_pos + replace_len);
+ geany_match_info_free(match);
}
else
{
View
4 src/document.h
@@ -34,6 +34,7 @@ G_BEGIN_DECLS
#include "Scintilla.h"
#include "ScintillaWidget.h"
#include "editor.h"
+#include "search.h"
#if defined(G_OS_WIN32)
# define GEANY_DEFAULT_EOL_CHARACTER SC_EOL_CRLF
@@ -222,7 +223,8 @@ gboolean document_search_bar_find(GeanyDocument *doc, const gchar *text, gint fl
gboolean backwards);
gint document_find_text(GeanyDocument *doc, const gchar *text, const gchar *original_text,
- gint flags, gboolean search_backwards, gboolean scroll, GtkWidget *parent);
+ gint flags, gboolean search_backwards, GeanyMatchInfo **match_,
+ gboolean scroll, GtkWidget *parent);
gint document_replace_text(GeanyDocument *doc, const gchar *find_text, const gchar *original_find_text,
const gchar *replace_text, gint flags, gboolean search_backwards);
View
182 src/search.c
@@ -420,7 +420,7 @@ void search_find_selection(GeanyDocument *doc, gboolean search_backwards)
{
setup_find_next(s); /* allow find next/prev */
- if (document_find_text(doc, s, NULL, 0, search_backwards, FALSE, NULL) > -1)
+ if (document_find_text(doc, s, NULL, 0, search_backwards, NULL, FALSE, NULL) > -1)
editor_display_current_line(doc->editor, 0.3F);
g_free(s);
}
@@ -1150,26 +1150,50 @@ on_find_replace_checkbutton_toggled(GtkToggleButton *togglebutton, gpointer user
}
+static GeanyMatchInfo *match_info_new(gint flags, gint start, gint end)
+{
+ GeanyMatchInfo *info = g_slice_alloc(sizeof *info);
+
+ info->flags = flags;
+ info->start = start;
+ info->end = end;
+ info->match_text = NULL;
+
+ return info;
+}
+
+void geany_match_info_free(GeanyMatchInfo *info)
+{
+ g_free(info->match_text);
+ g_slice_free1(sizeof *info, info);
+}
+
+
/* find all in the given range.
- * Returns a list of allocated Sci_TextToFind, should be freed using:
+ * Returns a list of allocated GeanyMatchInfo, should be freed using:
*
* foreach_slist(node, matches)
- * g_slice_free(struct Sci_TextToFind, node->data);
+ * geany_match_info_free(node->data);
* g_slist_free(matches); */
static GSList *find_range(ScintillaObject *sci, gint flags, struct Sci_TextToFind *ttf)
{
GSList *matches = NULL;
+ GeanyMatchInfo *info;
g_return_val_if_fail(sci != NULL && ttf->lpstrText != NULL, NULL);
if (! *ttf->lpstrText)
return NULL;
- while (search_find_text(sci, flags, ttf) != -1)
+ while (search_find_text(sci, flags, ttf, &info) != -1)
{
if (ttf->chrgText.cpMax > ttf->chrg.cpMax)
- break; /* found text is partially out of range */
+ {
+ /* found text is partially out of range */
+ geany_match_info_free(info);
+ break;
+ }
- matches = g_slist_prepend(matches, g_slice_copy(sizeof *ttf, ttf));
+ matches = g_slist_prepend(matches, info);
ttf->chrg.cpMin = ttf->chrgText.cpMax;
/* avoid rematching with empty matches like "(?=[a-z])" or "^$".
@@ -1206,16 +1230,13 @@ gint search_mark_all(GeanyDocument *doc, const gchar *search_text, gint flags)
matches = find_range(doc->editor->sci, flags, &ttf);
foreach_slist (match, matches)
{
- struct Sci_TextToFind *m = match->data;
+ GeanyMatchInfo *info = match->data;
- if (m->chrgText.cpMax != m->chrgText.cpMin)
- {
- editor_indicator_set_on_range(doc->editor, GEANY_INDICATOR_SEARCH,
- m->chrgText.cpMin, m->chrgText.cpMax);
- }
+ if (info->end != info->start)
+ editor_indicator_set_on_range(doc->editor, GEANY_INDICATOR_SEARCH, info->start, info->end);
count++;
- g_slice_free1(sizeof *m, m);
+ geany_match_info_free(info);
}
g_slist_free(matches);
@@ -1309,7 +1330,7 @@ on_find_dialog_response(GtkDialog *dialog, gint response, gpointer user_data)
case GEANY_RESPONSE_FIND_PREVIOUS:
{
gint result = document_find_text(doc, search_data.text, search_data.original_text, search_data.flags,
- (response == GEANY_RESPONSE_FIND_PREVIOUS), TRUE, GTK_WIDGET(find_dlg.dialog));
+ (response == GEANY_RESPONSE_FIND_PREVIOUS), NULL, TRUE, GTK_WIDGET(find_dlg.dialog));
ui_set_search_entry_background(find_dlg.entry, (result > -1));
check_close = search_prefs.hide_find_dialog;
break;
@@ -1451,7 +1472,7 @@ on_replace_dialog_response(GtkDialog *dialog, gint response, gpointer user_data)
search_backwards_re);
if (rep != -1)
document_find_text(doc, find, original_find, search_flags_re, search_backwards_re,
- TRUE, NULL);
+ NULL, TRUE, NULL);
break;
}
case GEANY_RESPONSE_REPLACE:
@@ -1462,7 +1483,7 @@ on_replace_dialog_response(GtkDialog *dialog, gint response, gpointer user_data)
case GEANY_RESPONSE_FIND:
{
gint result = document_find_text(doc, find, original_find, search_flags_re,
- search_backwards_re, TRUE, GTK_WIDGET(dialog));
+ search_backwards_re, NULL, TRUE, GTK_WIDGET(dialog));
ui_set_search_entry_background(replace_dlg.find_entry, (result > -1));
break;
}
@@ -1908,24 +1929,16 @@ static GRegex *compile_regex(const gchar *str, gint sflags)
}
-typedef struct CharOffsets
-{
- gint start, end;
-} CharOffsets;
-
-static CharOffsets regex_matches[10];
-
/* groups that don't exist are handled OK as len = end - start = (-1) - (-1) = 0 */
-static gchar *get_regex_match_string(const gchar *text, CharOffsets *match)
+static gchar *get_regex_match_string(const gchar *text, const GeanyMatchInfo *match, guint nth)
{
- return g_strndup(&text[match->start], match->end - match->start);
+ const gint start = match->matches[nth].start;
+ const gint end = match->matches[nth].end;
+ return g_strndup(&text[start], end - start);
}
-/* All matching text from regex_matches[0].start to regex_matches[0].end */
-static gchar *regex_match_text = NULL;
-
-static gint find_regex(ScintillaObject *sci, guint pos, GRegex *regex)
+static gint find_regex(ScintillaObject *sci, guint pos, GRegex *regex, GeanyMatchInfo *match)
{
const gchar *text;
GMatchInfo *minfo;
@@ -1933,9 +1946,6 @@ static gint find_regex(ScintillaObject *sci, guint pos, GRegex *regex)
g_return_val_if_fail(pos <= (guint)sci_get_length(sci), -1);
- /* clear old match */
- SETPTR(regex_match_text, NULL);
-
/* Warning: any SCI calls will invalidate 'text' after calling SCI_GETCHARACTERPOINTER */
text = (void*)scintilla_send_message(sci, SCI_GETCHARACTERPOINTER, 0, 0);
@@ -1945,57 +1955,87 @@ static gint find_regex(ScintillaObject *sci, guint pos, GRegex *regex)
guint i;
/* copy whole match text and offsets before they become invalid */
- regex_match_text = g_match_info_fetch(minfo, 0);
+ SETPTR(match->match_text, g_match_info_fetch(minfo, 0));
- foreach_range(i, G_N_ELEMENTS(regex_matches))
+ foreach_range(i, G_N_ELEMENTS(match->matches))
{
gint start = -1, end = -1;
g_match_info_fetch_pos(minfo, (gint)i, &start, &end);
- regex_matches[i].start = start;
- regex_matches[i].end = end;
+ match->matches[i].start = start;
+ match->matches[i].end = end;
}
- ret = regex_matches[0].start;
+ match->start = match->matches[0].start;
+ match->end = match->matches[0].end;
+ ret = match->start;
}
g_match_info_free(minfo);
return ret;
}
-gint search_find_next(ScintillaObject *sci, const gchar *str, gint flags)
+gint search_find_prev(ScintillaObject *sci, const gchar *str, gint flags, GeanyMatchInfo **match_)
+{
+ gint ret;
+
+ g_return_val_if_fail(! (flags & SCFIND_REGEXP), -1);
+
+ ret = sci_search_prev(sci, flags, str);
+ if (ret != -1 && match_)
+ *match_ = match_info_new(flags, ret, ret + strlen(str));
+ return ret;
+}
+
+
+gint search_find_next(ScintillaObject *sci, const gchar *str, gint flags, GeanyMatchInfo **match_)
{
+ GeanyMatchInfo *match;
GRegex *regex;
gint ret = -1;
gint pos;
if (~flags & SCFIND_REGEXP)
- return sci_search_next(sci, flags, str);
+ {
+ ret = sci_search_next(sci, flags, str);
+ if (ret != -1 && match_)
+ *match_ = match_info_new(flags, ret, ret + strlen(str));
+ return ret;
+ }
regex = compile_regex(str, flags);
if (!regex)
return -1;
+ match = match_info_new(flags, 0, 0);
+
pos = sci_get_current_position(sci);
- ret = find_regex(sci, pos, regex);
+ ret = find_regex(sci, pos, regex, match);
/* avoid re-matching the same position in case of empty matches */
- if (ret == pos && regex_matches[0].start == regex_matches[0].end)
- ret = find_regex(sci, pos + 1, regex);
+ if (ret == pos && match->matches[0].start == match->matches[0].end)
+ ret = find_regex(sci, pos + 1, regex, match);
if (ret >= 0)
- sci_set_selection(sci, ret, regex_matches[0].end);
+ sci_set_selection(sci, match->start, match->end);
+
+ if (ret != -1 && match_)
+ *match_ = match;
+ else
+ geany_match_info_free(match);
g_regex_unref(regex);
return ret;
}
-gint search_replace_target(ScintillaObject *sci, const gchar *replace_text,
- gboolean regex)
+gint search_replace_match(ScintillaObject *sci, const GeanyMatchInfo *match, const gchar *replace_text)
{
GString *str;
gint ret = 0;
gint i = 0;
- if (!regex)
+ sci_set_target_start(sci, match->start);
+ sci_set_target_end(sci, match->end);
+
+ if (! (match->flags & SCFIND_REGEXP))
return sci_replace_target(sci, replace_text, FALSE);
str = g_string_new(replace_text);
@@ -2021,8 +2061,7 @@ gint search_replace_target(ScintillaObject *sci, const gchar *replace_text,
/* digit escape */
g_string_erase(str, i, 2);
/* fix match offsets by subtracting index of whole match start from the string */
- grp = get_regex_match_string(regex_match_text - regex_matches[0].start,
- &regex_matches[c - '0']);
+ grp = get_regex_match_string(match->match_text - match->matches[0].start, match, c - '0');
g_string_insert(str, i, grp);
i += strlen(grp);
g_free(grp);
@@ -2033,29 +2072,40 @@ gint search_replace_target(ScintillaObject *sci, const gchar *replace_text,
}
-gint search_find_text(ScintillaObject *sci, gint flags, struct Sci_TextToFind *ttf)
+gint search_find_text(ScintillaObject *sci, gint flags, struct Sci_TextToFind *ttf, GeanyMatchInfo **match_)
{
+ GeanyMatchInfo *match = NULL;
GRegex *regex;
- gint pos;
gint ret;
if (~flags & SCFIND_REGEXP)
- return sci_find_text(sci, flags, ttf);
+ {
+ ret = sci_find_text(sci, flags, ttf);
+ if (ret != -1 && match_)
+ *match_ = match_info_new(flags, ttf->chrgText.cpMin, ttf->chrgText.cpMax);
+ return ret;
+ }
regex = compile_regex(ttf->lpstrText, flags);
if (!regex)
return -1;
- pos = ttf->chrg.cpMin;
- ret = find_regex(sci, pos, regex);
+ match = match_info_new(flags, 0, 0);
+ ret = find_regex(sci, ttf->chrg.cpMin, regex, match);
if (ret >= ttf->chrg.cpMax)
ret = -1;
else if (ret >= 0)
{
- ttf->chrgText.cpMin = regex_matches[0].start;
- ttf->chrgText.cpMax = regex_matches[0].end;
+ ttf->chrgText.cpMin = match->start;
+ ttf->chrgText.cpMax = match->end;
}
+
+ if (ret != -1 && match_)
+ *match_ = match;
+ else
+ geany_match_info_free(match);
+
g_regex_unref(regex);
return ret;
}
@@ -2080,8 +2130,8 @@ static gint find_document_usage(GeanyDocument *doc, const gchar *search_text, gi
matches = find_range(doc->editor->sci, flags, &ttf);
foreach_slist (match, matches)
{
- struct Sci_TextToFind *m = match->data;
- gint line = sci_get_line_from_position(doc->editor->sci, m->chrgText.cpMin);
+ GeanyMatchInfo *info = match->data;
+ gint line = sci_get_line_from_position(doc->editor->sci, info->start);
if (line != prev_line)
{
@@ -2093,7 +2143,7 @@ static gint find_document_usage(GeanyDocument *doc, const gchar *search_text, gi
}
count++;
- g_slice_free1(sizeof *m, m);
+ geany_match_info_free(info);
}
g_slist_free(matches);
g_free(short_file_name);
@@ -2169,24 +2219,24 @@ guint search_replace_range(ScintillaObject *sci, struct Sci_TextToFind *ttf,
matches = find_range(sci, flags, ttf);
foreach_slist (match, matches)
{
- struct Sci_TextToFind *m = match->data;
+ GeanyMatchInfo *info = match->data;
gint replace_len;
- sci_set_target_start(sci, offset + m->chrgText.cpMin);
- sci_set_target_end(sci, offset + m->chrgText.cpMax);
+ info->start += offset;
+ info->end += offset;
- replace_len = search_replace_target(sci, replace_text, flags & SCFIND_REGEXP);
- offset += replace_len - (m->chrgText.cpMax - m->chrgText.cpMin);
+ replace_len = search_replace_match(sci, info, replace_text);
+ offset += replace_len - (info->end - info->start);
count ++;
/* on last match, update the last match/new range end */
if (! match->next)
{
- ttf->chrg.cpMin = m->chrgText.cpMin;
+ ttf->chrg.cpMin = info->start;
ttf->chrg.cpMax += offset;
}
- g_slice_free1(sizeof *m, m);
+ geany_match_info_free(info);
}
g_slist_free(matches);
@@ -2204,7 +2254,7 @@ void search_find_again(gboolean change_direction)
{
gboolean forward = ! search_data.backwards;
gint result = document_find_text(doc, search_data.text, search_data.original_text, search_data.flags,
- change_direction ? forward : !forward, FALSE, NULL);
+ change_direction ? forward : !forward, NULL, FALSE, NULL);
if (result > -1)
editor_display_current_line(doc->editor, 0.3F);
View
27 src/search.h
@@ -68,6 +68,22 @@ GeanySearchPrefs;
extern GeanySearchPrefs search_prefs;
+typedef struct GeanyMatchInfo
+{
+ gint flags;
+ /* range */
+ gint start, end;
+ /* only valid if (flags & SCFIND_REGEX) */
+ gchar *match_text; /* text actually matched */
+ struct
+ {
+ gint start, end;
+ }
+ matches[10]; /* sub-patterns */
+}
+GeanyMatchInfo;
+
+
void search_init(void);
void search_finalize(void);
@@ -84,9 +100,13 @@ void search_show_find_in_files_dialog_full(const gchar *text, const gchar *dir);
struct _ScintillaObject;
struct Sci_TextToFind;
-gint search_find_next(struct _ScintillaObject *sci, const gchar *str, gint flags);
+void geany_match_info_free(GeanyMatchInfo *info);
+
+gint search_find_prev(struct _ScintillaObject *sci, const gchar *str, gint flags, GeanyMatchInfo **match_);
+
+gint search_find_next(struct _ScintillaObject *sci, const gchar *str, gint flags, GeanyMatchInfo **match_);
-gint search_find_text(struct _ScintillaObject *sci, gint flags, struct Sci_TextToFind *ttf);
+gint search_find_text(struct _ScintillaObject *sci, gint flags, struct Sci_TextToFind *ttf, GeanyMatchInfo **match_);
void search_find_again(gboolean change_direction);
@@ -96,8 +116,7 @@ void search_find_selection(GeanyDocument *doc, gboolean search_backwards);
gint search_mark_all(GeanyDocument *doc, const gchar *search_text, gint flags);
-gint search_replace_target(struct _ScintillaObject *sci, const gchar *replace_text,
- gboolean regex);
+gint search_replace_match(struct _ScintillaObject *sci, const GeanyMatchInfo *match, const gchar *replace_text);
guint search_replace_range(struct _ScintillaObject *sci, struct Sci_TextToFind *ttf,
gint flags, const gchar *replace_text);

0 comments on commit 5412a24

Please sign in to comment.
Something went wrong with that request. Please try again.