Subject: bug#46534: Lexical change in bindat breaks weechat.el

From: Stefan Monnier <[email-masked]>
Date: Mon, 15 Feb 2021 10:50:30 -0500

[ 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))