* [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails @ 2019-06-26 10:19 Kamil Dudka [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com> 2019-06-27 23:01 ` Oliver Kiddle 0 siblings, 2 replies; 8+ messages in thread From: Kamil Dudka @ 2019-06-26 10:19 UTC (permalink / raw) To: zsh-workers There is a bug report in Red Hat Bugzilla about zsh crashing on NULL pointer dereference: https://bugzilla.redhat.com/1722703 I was not able to reproduce the crash myself but the attached patch should prevent zsh from crashing in this situation. --- Src/Zle/zle_utils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c index 0277d4917..8081d3adc 100644 --- a/Src/Zle/zle_utils.c +++ b/Src/Zle/zle_utils.c @@ -1607,7 +1607,9 @@ static int unapplychange(struct change *ch) { if(ch->hist != histline) { - zle_setline(quietgethist(ch->hist)); + Histent he = quietgethist(ch->hist); + if(he) + zle_setline(he); zlecs = ch->new_cs; return 0; } @@ -1647,7 +1649,9 @@ static int applychange(struct change *ch) { if(ch->hist != histline) { - zle_setline(quietgethist(ch->hist)); + Histent he = quietgethist(ch->hist); + if(he) + zle_setline(he); zlecs = ch->old_cs; return 0; } -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <249696510.1723352.1561545091592@mail2.virginmedia.com>]
* Fwd: Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com> @ 2019-06-26 11:00 ` Peter Stephenson 2019-06-26 12:07 ` Kamil Dudka 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2019-06-26 11:00 UTC (permalink / raw) To: zsh workers Sorry, this should have gone to the list... ---------- Original Message ---------- From: Peter Stephenson <p.w.stephenson@ntlworld.com> To: Kamil Dudka <kdudka@redhat.com> Date: 26 June 2019 at 11:31 Subject: Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails > On 26 June 2019 at 11:19 Kamil Dudka <kdudka@redhat.com> wrote: > > > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL > pointer dereference: https://bugzilla.redhat.com/1722703 > > I was not able to reproduce the crash myself but the attached patch > should prevent zsh from crashing in this situation. Hmm... I'm guessing that in the failure case we probably shouldn't set zlecs either? It's probably not going to do anything helpful. Possibly also return 1? pws > Src/Zle/zle_utils.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c > index 0277d4917..8081d3adc 100644 > --- a/Src/Zle/zle_utils.c > +++ b/Src/Zle/zle_utils.c > @@ -1607,7 +1607,9 @@ static int > unapplychange(struct change *ch) > { > if(ch->hist != histline) { > - zle_setline(quietgethist(ch->hist)); > + Histent he = quietgethist(ch->hist); > + if(he) > + zle_setline(he); > zlecs = ch->new_cs; > return 0; > } > @@ -1647,7 +1649,9 @@ static int > applychange(struct change *ch) > { > if(ch->hist != histline) { > - zle_setline(quietgethist(ch->hist)); > + Histent he = quietgethist(ch->hist); > + if(he) > + zle_setline(he); > zlecs = ch->old_cs; > return 0; > } > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails 2019-06-26 11:00 ` Fwd: " Peter Stephenson @ 2019-06-26 12:07 ` Kamil Dudka 0 siblings, 0 replies; 8+ messages in thread From: Kamil Dudka @ 2019-06-26 12:07 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On Wednesday, June 26, 2019 1:00:44 PM CEST Peter Stephenson wrote: > > On 26 June 2019 at 11:19 Kamil Dudka <kdudka@redhat.com> wrote: > > > > > > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL > > pointer dereference: https://bugzilla.redhat.com/1722703 > > > > I was not able to reproduce the crash myself but the attached patch > > should prevent zsh from crashing in this situation. > > Hmm... I'm guessing that in the failure case we probably shouldn't > set zlecs either? It's probably not going to do anything helpful. > Possibly also return 1? > > pws Good point. zlecs should not be updated when zleline is not updated. Sorry for missing it in the proposed patch. Returning 1 might also be better although I do not really understand why quietgethist() returns NULL in the mentioned case. Kamil ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails 2019-06-26 10:19 [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails Kamil Dudka [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com> @ 2019-06-27 23:01 ` Oliver Kiddle 2019-06-28 0:20 ` Mikael Magnusson ` (2 more replies) 1 sibling, 3 replies; 8+ messages in thread From: Oliver Kiddle @ 2019-06-27 23:01 UTC (permalink / raw) To: Kamil Dudka; +Cc: zsh-workers Kamil Dudka wrote: > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL > pointer dereference: https://bugzilla.redhat.com/1722703 > > I was not able to reproduce the crash myself but the attached patch > should prevent zsh from crashing in this situation. It'd perhaps be good to add a DPUTS call here because while it is good not to crash the condition does perhaps indicate a bug and it'd be better not to hide it completely. I can't reproduce the crash either but there must be some plugins or url quoting magic going on here. More details of the setup used by the user might help find the underlying problem. Oliver ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails 2019-06-27 23:01 ` Oliver Kiddle @ 2019-06-28 0:20 ` Mikael Magnusson 2019-06-28 8:12 ` Kamil Dudka 2019-07-23 13:45 ` [PATCH v2] " Kamil Dudka 2 siblings, 0 replies; 8+ messages in thread From: Mikael Magnusson @ 2019-06-28 0:20 UTC (permalink / raw) To: Oliver Kiddle; +Cc: Kamil Dudka, zsh-workers I can reproduce *a* crash, not necessarily *the* crash, by making a 2x2 window and then maximizing/unmaximizing the window. It seems to not reproduce if I have run any commands in the shell. On 6/28/19, Oliver Kiddle <okiddle@yahoo.co.uk> wrote: > Kamil Dudka wrote: >> There is a bug report in Red Hat Bugzilla about zsh crashing on NULL >> pointer dereference: https://bugzilla.redhat.com/1722703 >> >> I was not able to reproduce the crash myself but the attached patch >> should prevent zsh from crashing in this situation. > > It'd perhaps be good to add a DPUTS call here because while it is good > not to crash the condition does perhaps indicate a bug and it'd be > better not to hide it completely. I can't reproduce the crash either but > there must be some plugins or url quoting magic going on here. > More details of the setup used by the user might help find the > underlying problem. > > Oliver > -- Mikael Magnusson ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails 2019-06-27 23:01 ` Oliver Kiddle 2019-06-28 0:20 ` Mikael Magnusson @ 2019-06-28 8:12 ` Kamil Dudka 2019-07-23 13:45 ` [PATCH v2] " Kamil Dudka 2 siblings, 0 replies; 8+ messages in thread From: Kamil Dudka @ 2019-06-28 8:12 UTC (permalink / raw) To: Oliver Kiddle; +Cc: zsh-workers On Friday, June 28, 2019 1:01:25 AM CEST Oliver Kiddle wrote: > Kamil Dudka wrote: > > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL > > pointer dereference: https://bugzilla.redhat.com/1722703 > > > > I was not able to reproduce the crash myself but the attached patch > > should prevent zsh from crashing in this situation. > > It'd perhaps be good to add a DPUTS call here because while it is good > not to crash the condition does perhaps indicate a bug and it'd be > better not to hide it completely. I can't reproduce the crash either but > there must be some plugins or url quoting magic going on here. > More details of the setup used by the user might help find the > underlying problem. > > Oliver Thank you for analyzing it! I do not have any more information than what is available in the mentioned bugzilla. So I asked the reporter about their setup. Kamil ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails 2019-06-27 23:01 ` Oliver Kiddle 2019-06-28 0:20 ` Mikael Magnusson 2019-06-28 8:12 ` Kamil Dudka @ 2019-07-23 13:45 ` Kamil Dudka 2019-07-26 15:24 ` Kamil Dudka 2 siblings, 1 reply; 8+ messages in thread From: Kamil Dudka @ 2019-07-23 13:45 UTC (permalink / raw) To: Oliver Kiddle; +Cc: Peter Stephenson, zsh-workers There is a bug report in Red Hat Bugzilla about zsh crashing on NULL pointer dereference: https://bugzilla.redhat.com/1722703 I was not able to reproduce the crash myself but the attached patch should prevent zsh from crashing in this situation. --- Src/Zle/zle_utils.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c index 0277d4917..d549b885b 100644 --- a/Src/Zle/zle_utils.c +++ b/Src/Zle/zle_utils.c @@ -1607,7 +1607,12 @@ static int unapplychange(struct change *ch) { if(ch->hist != histline) { - zle_setline(quietgethist(ch->hist)); + Histent he = quietgethist(ch->hist); + if(!he) { + dputs(ERRMSG("quietgethist(ch->hist) returned NULL")); + return 1; + } + zle_setline(he); zlecs = ch->new_cs; return 0; } @@ -1647,7 +1652,12 @@ static int applychange(struct change *ch) { if(ch->hist != histline) { - zle_setline(quietgethist(ch->hist)); + Histent he = quietgethist(ch->hist); + if(!he) { + dputs(ERRMSG("quietgethist(ch->hist) returned NULL")); + return 1; + } + zle_setline(he); zlecs = ch->old_cs; return 0; } -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails 2019-07-23 13:45 ` [PATCH v2] " Kamil Dudka @ 2019-07-26 15:24 ` Kamil Dudka 0 siblings, 0 replies; 8+ messages in thread From: Kamil Dudka @ 2019-07-26 15:24 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers, Oliver Kiddle [-- Attachment #1: Type: text/plain, Size: 1444 bytes --] Sorry, I used dputs() incorrectly, which caused undefined symbols in a non-debug build. The attached patch fixes it. Sorry for the troubles! Kamil On Tuesday, July 23, 2019 3:45:48 PM CEST Kamil Dudka wrote: > There is a bug report in Red Hat Bugzilla about zsh crashing on NULL > pointer dereference: https://bugzilla.redhat.com/1722703 > > I was not able to reproduce the crash myself but the attached patch > should prevent zsh from crashing in this situation. > --- > Src/Zle/zle_utils.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c > index 0277d4917..d549b885b 100644 > --- a/Src/Zle/zle_utils.c > +++ b/Src/Zle/zle_utils.c > @@ -1607,7 +1607,12 @@ static int > unapplychange(struct change *ch) > { > if(ch->hist != histline) { > - zle_setline(quietgethist(ch->hist)); > + Histent he = quietgethist(ch->hist); > + if(!he) { > + dputs(ERRMSG("quietgethist(ch->hist) returned NULL")); > + return 1; > + } > + zle_setline(he); > zlecs = ch->new_cs; > return 0; > } > @@ -1647,7 +1652,12 @@ static int > applychange(struct change *ch) > { > if(ch->hist != histline) { > - zle_setline(quietgethist(ch->hist)); > + Histent he = quietgethist(ch->hist); > + if(!he) { > + dputs(ERRMSG("quietgethist(ch->hist) returned NULL")); > + return 1; > + } > + zle_setline(he); > zlecs = ch->old_cs; > return 0; > } [-- Attachment #2: 0001-zle_utils-fix-incorrect-use-of-dputs.patch --] [-- Type: text/x-patch, Size: 1210 bytes --] From 97c195f6de8c566e88c0bb753aadc8217dc37875 Mon Sep 17 00:00:00 2001 From: Kamil Dudka <kdudka@redhat.com> Date: Fri, 26 Jul 2019 17:17:40 +0200 Subject: [PATCH] zle_utils: fix incorrect use of dputs() ... that caused undefined symbols in a non-debug build --- Src/Zle/zle_utils.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c index d549b885b..2b306fdcd 100644 --- a/Src/Zle/zle_utils.c +++ b/Src/Zle/zle_utils.c @@ -1608,10 +1608,9 @@ unapplychange(struct change *ch) { if(ch->hist != histline) { Histent he = quietgethist(ch->hist); - if(!he) { - dputs(ERRMSG("quietgethist(ch->hist) returned NULL")); + DPUTS(he == NULL, "quietgethist(ch->hist) returned NULL"); + if(he == NULL) return 1; - } zle_setline(he); zlecs = ch->new_cs; return 0; @@ -1653,10 +1652,9 @@ applychange(struct change *ch) { if(ch->hist != histline) { Histent he = quietgethist(ch->hist); - if(!he) { - dputs(ERRMSG("quietgethist(ch->hist) returned NULL")); + DPUTS(he == NULL, "quietgethist(ch->hist) returned NULL"); + if(he == NULL) return 1; - } zle_setline(he); zlecs = ch->old_cs; return 0; -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-26 15:25 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-26 10:19 [PATCH] {,un}applychange: do not call zle_setline(NULL) if quietgethist() fails Kamil Dudka [not found] ` <249696510.1723352.1561545091592@mail2.virginmedia.com> 2019-06-26 11:00 ` Fwd: " Peter Stephenson 2019-06-26 12:07 ` Kamil Dudka 2019-06-27 23:01 ` Oliver Kiddle 2019-06-28 0:20 ` Mikael Magnusson 2019-06-28 8:12 ` Kamil Dudka 2019-07-23 13:45 ` [PATCH v2] " Kamil Dudka 2019-07-26 15:24 ` Kamil Dudka
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).