Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed a crash of geany by the git-changebar on SCI_ADDTEXT #1106

Merged
merged 2 commits into from Sep 11, 2021
Merged

Fixed a crash of geany by the git-changebar on SCI_ADDTEXT #1106

merged 2 commits into from Sep 11, 2021

Conversation

vlvlbel
Copy link
Contributor

@vlvlbel vlvlbel commented Sep 3, 2021

Fixed a crash of geany by the git-changebar plugin by changing a glong type to sptr_t in the SCI_ADDTEXT invoking (MinGW64 build)

…g type to sptr_t in the SCI_ADDTEXT invoking (MinGW64 build)
@vlvlbel
Copy link
Contributor Author

vlvlbel commented Sep 3, 2021

Hi guys.

I had trouble with the git-changebar plugin, which crashed Geany on SCI_ADDTEXT each time until I changed the glong type casting to sptr_t, like it is used with SCI_ADDTEXT in other places.

I build the geany with MinGW64, so the geany and the scintilla are 64 bit.

Regards,
Vlad.

@elextr
Copy link
Member

elextr commented Sep 3, 2021

Makes sense, Win64 is the only platform where sizeof(long) != sizeof(ptr) ping @b4n

@eht16
Copy link
Member

eht16 commented Sep 4, 2021

Tested on Windows, also against a x86_64 build and works.

However, Geany didn't crash on my system without the change. I still consider the change as correct.

@vlvlbel for my curiosity, how do you build Geany? I assume in a Mingw64 shell against "mingw-w64-x86_64-*" packages? I'm wondering why it didn't crash for me.

For completeness, there are a couple of more occurences with the wrong cast:

geanylua/glspi_sci.c:                   if (len) { scintilla_send_message(doc->editor->sci,SCI_COPYTEXT,len,(glong)content); }
geanymacro/src/geanymacro.c:                    scintilla_send_message(sci,me->message,me->wparam,(glong)clipboardcontents);
geanymacro/src/geanymacro.c:                    me->lparam=(glong)(g_strcompress(s[(*k)++]));
geanymacro/src/geanymacro.c:                            me->lparam=(glong)NULL;
geanymacro/src/geanymacro.c:                    me->lparam=(glong)(g_strcompress(s[(*k)++]));
geanymacro/src/geanymacro.c:            ?((glong) g_strdup((gchar *)(nt->lParam))) : nt->lParam;
geanymacro/src/geanymacro.c:                                    me->lparam=(glong)((cTemp!=NULL)?g_strdup(cTemp):g_strdup(""));
geanymacro/src/geanymacro.c:                                    me->lparam=(glong)(((*cTemp2)==0)?NULL:g_strdup(cTemp2));
geanynumberedbookmarks/src/geanynumberedbookmarks.c:            scintilla_send_message(sci,SCI_MARKERDEFINEPIXMAP,m,(glong)(aszMarkerImages[k]));
git-changebar/src/gcb-plugin.c:  scintilla_send_message (sci, SCI_ADDTEXT, buf_len, (glong) buf);
git-changebar/src/gcb-plugin.c:  scintilla_send_message (old_sci, SCI_ADDTEXT, old_buf_len, (glong) old_buf);

Should we fix those as well?

@eht16 eht16 added this to the 1.38.0 milestone Sep 4, 2021
@vlvlbel
Copy link
Contributor Author

vlvlbel commented Sep 4, 2021

how do you build Geany? I assume in a Mingw64 shell against "mingw-w64-x86_64-*" packages? I'm wondering why it didn't crash for me.

@eht16 yeah, I have built Geany from Mingw64 shell against /mingw64/lib and /mingw64/include, i.e against mingw-w64-x86_64 packages. I recently changed my MSYS2 environment, previously it had been x32 MSYS2, now it is x64 MSYS2 fresh installed, so I had to rebuilt my Geany instance. The previous Geany instance was x64 with x32 Scintilla built as DLL (few years ago there were troubles building x64 Scintilla).

For completeness, there are a couple of more occurences with the wrong cast:
Should we fix those as well?

I think these occurrences should be fixed as well. The numbered bookmarks plugin crashed for me also, but I just switched it off, and I don't use the Lua plugin and Geanymacro yet. Git-changebar is the only crucial plugin for me in the list above.

@eht16
Copy link
Member

eht16 commented Sep 4, 2021

how do you build Geany? I assume in a Mingw64 shell against "mingw-w64-x86_64-*" packages? I'm wondering why it didn't crash for me.

@eht16 yeah, I have built Geany from Mingw64 shell against /mingw64/lib and /mingw64/include, i.e against mingw-w64-x86_64 packages. I recently changed my MSYS2 environment, previously it had been x32 MSYS2, now it is x64 MSYS2 fresh installed, so I had to rebuilt my Geany instance. The previous Geany instance was x64 with x32

That might be the difference, I use the same combination as you (Mingw64 shell + /mingw64/lib) but my MSYS2 installation is rather old.
Thanks for the information.

For completeness, there are a couple of more occurences with the wrong cast:
Should we fix those as well?

I think these occurrences should be fixed as well. The numbered bookmarks plugin crashed for me also, [...]

Agreed, would you mind extending this PR?

@vlvlbel
Copy link
Contributor Author

vlvlbel commented Sep 4, 2021

Agreed, would you mind extending this PR?

@eht16 yeah, sure, I will do it on Monday.

…4 platform due to a different size of glong and sptr_t
@vlvlbel
Copy link
Contributor Author

vlvlbel commented Sep 6, 2021

@eht16 I've added the second commit to this pull request.

I've tested Macro and Numbered Bookmarks Plugins - after the changes they began to work for me (before the changes they crashed Geany).

I haven't tested the Lua plugin, but due to a small change there, I believe, it should work.

@eht16
Copy link
Member

eht16 commented Sep 6, 2021

@vlvlbel thanks!
LGTM.
I'm going to merge this in a few days, after CI is working again.

@eht16 eht16 merged commit ddf06e8 into geany:master Sep 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants