GNU bug report logs - #46534
Lexical change in bindat breaks weechat.el

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Tony Olagbaiye <me <at> fron.io>
Subject: bug#46534: closed (Re: bug#46534: Lexical change in bindat breaks
 weechat.el)
Date: Mon, 15 Feb 2021 19:52:03 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#46534: Lexical change in bindat breaks weechat.el

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 46534 <at> debbugs.gnu.org.

-- 
46534: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=46534
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Tony Olagbaiye <me <at> fron.io>
Cc: storm <at> cua.dk, 46534-done <at> debbugs.gnu.org
Subject: Re: bug#46534: Lexical change in bindat breaks weechat.el
Date: Mon, 15 Feb 2021 14:51:29 -0500

> Indeed, I can confirm this fixes the test file as well as weechat.el proper, thanks!

Thanks, closing for now, tho I'd love to see a better fix
(e.g. changing the system so it doesn't rely on `eval` and dynamic
scoping so much).


        Stefan


> Sent from ProtonMail mobile
>
> -------- Original Message --------
> On 15 Feb 2021, 15:50, Stefan Monnier < monnier <at> iro.umontreal.ca> wrote:
>
>  [ Hi Kim, long time no see.
>  I'd appreciate your opinion on this issue with bindat.el. ]
>
>  > (defconst minrepro--str-spec
>  > '((len u32)
>  > (val str (eval (let ((len (minrepro--bindat-unsigned-to-signed
>  > (bindat-get-field struct 'len)
>  > 4)))
>  > ;; Hack for signed/unsigned problems
>  > (if (<= len 0) 0 len))))))
>
>  Hmm... the doc of bindat.el does not include `struct` among the vars you
>  can use in `eval`.
>
>  OTOH, a variable which you can use is `last` and it indeed contains
>  exactly the info you need from `struct`, so you can rewrite the above to:
>
>  (defconst minrepro--str-spec
>  '((len u32)
>  (val str (eval (let ((len (minrepro--bindat-unsigned-to-signed
>  last 4)))
>  ;; Hack for signed/unsigned problems
>  (if (<= len 0) 0 len))))))
>
>  > (defconst minrepro--message-spec
>  > '((length u32)
>  > (compression u8)
>  > (id struct minrepro--str-spec)
>  > (data vec (eval (let ((l (- (bindat-get-field struct 'length)
>  > 4 ;length
>  > 1 ;compression
>  > (+ 4 (length (bindat-get-field struct 'id 'val))))))
>  > l)))))
>
>  This one OTOH can't just use `last` since that only gives us the `id`
>  field but not the `length` field :-(
>
>  I can't see any way to do what you want given the documentation found in
>  the `Commentary:` of `bindat.el`, so I guess we do need to extend the
>  documented functionality.
>
>  I installed the patch below, for now. It fixes the problem in your test
>  case and hopefully in other cases as well. Please confirm.
>
>  Stefan
>
>  diff --git a/lisp/emacs-lisp/bindat.el b/lisp/emacs-lisp/bindat.el
>  index 0d9ba57d66..bf01347ae0 100644
>  --- a/lisp/emacs-lisp/bindat.el
>  +++ b/lisp/emacs-lisp/bindat.el
>  @@ -26,7 +26,7 @@
>  ;; Packing and unpacking of (binary) data structures.
>  ;;
>  ;; The data formats used in binary files and network protocols are
>  -;; often structed data which can be described by a C-style structure
>  +;; often structured data which can be described by a C-style structure
>  ;; such as the one shown below. Using the bindat package, decoding
>  ;; and encoding binary data formats like these is made simple using a
>  ;; structure specification which closely resembles the C style
>  @@ -135,7 +135,8 @@
>  ;; | ( [FIELD] repeat COUNT ITEM... )
>
>  ;; -- In (eval EXPR), the value of the last field is available in
>  -;; the dynamically bound variable `last'.
>  +;; the dynamically bound variable `last' and all the previous
>  +;; ones in the variable `struct'.
>
>  ;; TYPE ::= ( eval EXPR ) -- interpret result as TYPE
>  ;; | u8 | byte -- length 1
>  @@ -191,7 +192,7 @@
>  ;;; Code:
>
>  ;; Helper functions for structure unpacking.
>  -;; Relies on dynamic binding of BINDAT-RAW and BINDAT-IDX
>  +;; Relies on dynamic binding of `bindat-raw' and `bindat-idx'.
>
>  (defvar bindat-raw)
>  (defvar bindat-idx)
>  @@ -276,8 +277,8 @@ bindat--unpack-item
>  (t nil)))
>
>  (defun bindat--unpack-group (spec)
>  - (with-suppressed-warnings ((lexical last))
>  - (defvar last))
>  + (with-suppressed-warnings ((lexical struct last))
>  + (defvar struct) (defvar last))
>  (let (struct last)
>  (while spec
>  (let* ((item (car spec))
>  @@ -378,9 +379,9 @@ bindat--fixed-length-alist
>  (ip . 4)))
>
>  (defun bindat--length-group (struct spec)
>  - (with-suppressed-warnings ((lexical last))
>  - (defvar last))
>  - (let (last)
>  + (with-suppressed-warnings ((lexical struct last))
>  + (defvar struct) (defvar last))
>  + (let ((struct struct) last)
>  (while spec
>  (let* ((item (car spec))
>  (field (car item))
>  @@ -544,9 +545,9 @@ bindat--pack-item
>  (setq bindat-idx (+ bindat-idx len)))))
>
>  (defun bindat--pack-group (struct spec)
>  - (with-suppressed-warnings ((lexical last))
>  - (defvar last))
>  - (let (last)
>  + (with-suppressed-warnings ((lexical struct last))
>  + (defvar struct) (defvar last))
>  + (let ((struct struct) last)
>  (while spec
>  (let* ((item (car spec))
>  (field (car item))

[Message part 3 (message/rfc822, inline)]

From: Tony Olagbaiye <me <at> fron.io>
To: bug-gnu-emacs <at> gnu.org
Cc: monnier <monnier <at> iro.umontreal.ca>
Subject: Lexical change in bindat breaks weechat.el
Date: Mon, 15 Feb 2021 14:29:06 +0000

[Message part 4 (text/plain, inline)]

Hi,

As of commit c8c4d65d6510724acd40527a9af67e21e3cf4d5e (as bisected in my stead by wasamasa on \#emacs) it seems bindat changes have broken weechat.el, or specifically the weechat-relay module.

Attached a minimal reproducible script which fails on master but succeeds prior to the mentioned commit: weechat-bindat.bug.el.txt

Backtrace:

Debugger entered--Lisp error: (void-variable struct) (bindat-get-field struct 'len) (weechat--relay-bindat-unsigned-to-signed (bindat-get-field struct 'len) 4) (let ((len (weechat--relay-bindat-unsigned-to-signed (bindat-get-field struct 'len) 4))) (if (<= len 0) 0 len)) bindat--unpack-group(((len u32) (val str (eval (let ((len (weechat--relay-bindat-unsigned-to-signed ... 4))) (if (<= len 0) 0 len)))))) bindat--unpack-group(((length u32) (compression u8) (id struct weechat--relay-str-spec) (data vec (eval (let ((l (- ... 4 1 ...))) l))))) bindat-unpack(((length u32) (compression u8) (id struct weechat--relay-str-spec) (data vec (eval (let ((l (- ... 4 1 ...))) l)))) "\\0\\0\\0$\\0\\0\\0\\0\\4G255inf\\0\\0\\0\\7version\\0\\0\\0\\0053.0.1") weechat-unpack-message("\\0\\0\\0$\\0\\0\\0\\0\\4G255inf\\0\\0\\0\\7version\\0\\0\\0\\0053.0.1") weechat--relay-parse-new-message() weechat--relay-process-filter(\#<process weechat-relay-tls> "\\0\\0\\0$\\0\\0\\0\\0\\4G255inf\\0\\0\\0\\7version\\0\\0\\0\\0053.0.1")

Best,
bqv

[Message part 5 (text/html, inline)]

[weechat-bindat.bug.el.txt (text/plain, attachment)]

[publickey - EmailAddress(s=me@fron.io) - 0x3026807C.asc (application/pgp-keys, attachment)]

[signature.asc (application/pgp-signature, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.