From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Tony Olagbaiye <me@fron.io>
Cc: 46534-done@debbugs.gnu.org, storm@cua.dk
Subject: bug#46534: Lexical change in bindat breaks weechat.el
Date: Mon, 15 Feb 2021 14:51:29 -0500 [thread overview]
Message-ID: <jwvmtw5p0tz.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <kjmaTzHkRZuk__kOISktm49Ri1TIgrjjROkwMJ6RAsan_ZNczNMbjyXHzQJfM8v4RwHpk9578MCRmv1UnkXqHw==@fron.io> (Tony Olagbaiye's message of "Mon, 15 Feb 2021 19:19:54 +0000")
> 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@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))
next prev parent reply other threads:[~2021-02-15 19:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-15 14:29 bug#46534: Lexical change in bindat breaks weechat.el Tony Olagbaiye
2021-02-15 15:50 ` Stefan Monnier
2021-02-15 19:19 ` Tony Olagbaiye
2021-02-15 19:51 ` Stefan Monnier [this message]
2021-02-17 22:47 ` Kim Storm
2021-02-18 16:09 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvmtw5p0tz.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=46534-done@debbugs.gnu.org \
--cc=me@fron.io \
--cc=storm@cua.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).