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

[Tony Olagbaiye]

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


Sent from ProtonMail mobile



-------- Original Message --------
On 15 Feb 2021, 15:50, Stefan Monnier < monnier@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))

publickey - EmailAddress(s=me@fron.io) - 0x3026807C.asc
Description: application/pgp-keys

signature.asc
Description: OpenPGP digital signature