bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Alex Gramiak
frame-size-history can be nil, which resulted in an infloop.


0001-lisp-frame.el-frame-size-history-Fix-infloop.patch (845 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Eli Zaretskii
> From: Alex Gramiak <[hidden email]>
> Date: Sun, 14 Apr 2019 09:37:10 -0600
>
> frame-size-history can be nil, which resulted in an infloop.
>
> >From 5695525ed1477e908536d159da670f0f1ab4a369 Mon Sep 17 00:00:00 2001
> From: Alexander Gramiak <[hidden email]>
> Date: Sun, 14 Apr 2019 09:27:50 -0600
> Subject: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.
>
> ---
>  lisp/frame.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/frame.el b/lisp/frame.el
> index b5c936a51e..6f9e769e16 100644
> --- a/lisp/frame.el
> +++ b/lisp/frame.el
> @@ -1610,7 +1610,7 @@ frame--size-history
>      (with-current-buffer (get-buffer-create "*frame-size-history*")
>        (erase-buffer)
>        (insert (format "Frame size history of %s\n" frame))
> -      (while (listp (setq entry (pop history)))
> +      (while (consp (setq entry (pop history)))
>   (when (eq (car entry) frame)
>            (pop entry)
>            (insert (format "%s" (pop entry)))

Thanks.

But wouldn't it be better to special-case the nil value, and display
something to that effect, instead of just nothing after the header?

P.S. Regardless, please be sure to mention the bug number when you
push, as it is not in the patch you sent.



Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Alex Gramiak
Eli Zaretskii <[hidden email]> writes:

>> From: Alex Gramiak <[hidden email]>
>> Date: Sun, 14 Apr 2019 09:37:10 -0600
>>
>> frame-size-history can be nil, which resulted in an infloop.
>>
>> >From 5695525ed1477e908536d159da670f0f1ab4a369 Mon Sep 17 00:00:00 2001
>> From: Alexander Gramiak <[hidden email]>
>> Date: Sun, 14 Apr 2019 09:27:50 -0600
>> Subject: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.
>>
>> ---
>>  lisp/frame.el | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lisp/frame.el b/lisp/frame.el
>> index b5c936a51e..6f9e769e16 100644
>> --- a/lisp/frame.el
>> +++ b/lisp/frame.el
>> @@ -1610,7 +1610,7 @@ frame--size-history
>>      (with-current-buffer (get-buffer-create "*frame-size-history*")
>>        (erase-buffer)
>>        (insert (format "Frame size history of %s\n" frame))
>> -      (while (listp (setq entry (pop history)))
>> +      (while (consp (setq entry (pop history)))
>>   (when (eq (car entry) frame)
>>            (pop entry)
>>            (insert (format "%s" (pop entry)))
>
> Thanks.
>
> But wouldn't it be better to special-case the nil value, and display
> something to that effect, instead of just nothing after the header?
I suppose so. Here's a version which does that:


0001-lisp-frame.el-frame-size-history-Fix-infloop.-Bug-35.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Basil L. Contovounesios
Alex Gramiak <[hidden email]> writes:

> +      (when (null frame-size-history)
> +        (insert "No frame size history available.\n")))))

AKA (unless frame-size-history ...), if you prefer.

Thanks,

--
Basil



Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Eli Zaretskii
In reply to this post by Alex Gramiak
> From: Alex Gramiak <[hidden email]>
> Cc: [hidden email]
> Date: Sun, 14 Apr 2019 10:25:42 -0600
>
> +      (when (null frame-size-history)
> +        (insert "No frame size history available.\n")))))

LGTM, but maybe say "Frame size history is nil" or some such.

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Alex Gramiak
In reply to this post by Basil L. Contovounesios
close 35272
quit

"Basil L. Contovounesios" <[hidden email]> writes:

> Alex Gramiak <[hidden email]> writes:
>
>> +      (when (null frame-size-history)
>> +        (insert "No frame size history available.\n")))))
>
> AKA (unless frame-size-history ...), if you prefer.

Of course, I just felt like being more explicit when I made the commit.
Though unless is generally better, so I used changed the patch to use
it.

It's pushed as e233dedde to master.

P.S. Eli, is there going to be an Emacs 26.3? If so, could this patch
and perhaps the GTK memory leak patches be applied to emacs-26?



Reply | Threaded
Open this post in threaded view
|

bug#35272: [PATCH] * lisp/frame.el (frame--size-history): Fix infloop.

Eli Zaretskii
> From: Alex Gramiak <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>,  [hidden email]
> Date: Sun, 14 Apr 2019 11:43:56 -0600
>
> P.S. Eli, is there going to be an Emacs 26.3?

I don't know yet.

> If so, could this patch and perhaps the GTK memory leak patches be
> applied to emacs-26?

Yes, you can backport them.