Inadequate coding in hack-elisp-shorthands

Inadequate coding in hack-elisp-shorthands

[Alan Mackenzie]

Hello, Emacs.

In emacs -Q in the emacs-28 branch, create the following two line file,
foobar.el, and try to load it:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defvar foo-baz "foobar-baz")
FOOBARELISP-SHORTHANDS: (("foo" . "foobar")))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

This will throw an error, but that isn't important.

What is important is that the symbol foobar-baz is created by the
elisp-shorthands facility.

This shouldn't happen since:
1/- There is no Local Variables section.
2/- There is no variable elisp-shorthands in that non-existent section.

The following errors are evident in hack-elisp-shorthands:
1/- The code doesn't check for a correctly formatted Local Variables
  section.
2/- The code, even if it did check, would only check the last 3000 bytes
  in the file.  The section can occur anywhere in the last 3000
  CHARACTERS.
3/- The code doesn't do a case-sensitive search for "elisp-shorthands".
4/- The code doesn't check for "elisp-shorthands" being a complete
  symbol.
5/- The code doesn't even check that "elisp-shorthands" is in a comment.

I would suggest that these errors be corrected.  I would also suggest
that the entire code and documentation for this new facility be
carefully reviewed by somebody who isn't the original author.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Inadequate coding in hack-elisp-shorthands

[Eli Zaretskii]

> Date: Thu, 30 Sep 2021 20:35:17 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> In emacs -Q in the emacs-28 branch, create the following two line file,
> foobar.el, and try to load it:
> 
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> (defvar foo-baz "foobar-baz")
> FOOBARELISP-SHORTHANDS: (("foo" . "foobar")))
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> 
> This will throw an error, but that isn't important.
> 
> What is important is that the symbol foobar-baz is created by the
> elisp-shorthands facility.
> 
> This shouldn't happen since:
> 1/- There is no Local Variables section.
> 2/- There is no variable elisp-shorthands in that non-existent section.
> 
> The following errors are evident in hack-elisp-shorthands:
> 1/- The code doesn't check for a correctly formatted Local Variables
>   section.
> 2/- The code, even if it did check, would only check the last 3000 bytes
>   in the file.  The section can occur anywhere in the last 3000
>   CHARACTERS.
> 3/- The code doesn't do a case-sensitive search for "elisp-shorthands".
> 4/- The code doesn't check for "elisp-shorthands" being a complete
>   symbol.
> 5/- The code doesn't even check that "elisp-shorthands" is in a comment.
> 
> I would suggest that these errors be corrected.  I would also suggest
> that the entire code and documentation for this new facility be
> carefully reviewed by somebody who isn't the original author.

Thanks, but why isn't this a full-blown bug report, submitted to
debbugs?

Re: Inadequate coding in hack-elisp-shorthands

[Alan Mackenzie]

Hello, Eli.

On Fri, Oct 01, 2021 at 08:51:18 +0300, Eli Zaretskii wrote:
> > Date: Thu, 30 Sep 2021 20:35:17 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > In emacs -Q in the emacs-28 branch, create the following two line file,
> > foobar.el, and try to load it:

> > ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> > (defvar foo-baz "foobar-baz")
> > FOOBARELISP-SHORTHANDS: (("foo" . "foobar")))
> > ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> > This will throw an error, but that isn't important.

> > What is important is that the symbol foobar-baz is created by the
> > elisp-shorthands facility.

> > This shouldn't happen since:
> > 1/- There is no Local Variables section.
> > 2/- There is no variable elisp-shorthands in that non-existent section.

> > The following errors are evident in hack-elisp-shorthands:
> > 1/- The code doesn't check for a correctly formatted Local Variables
> >   section.
> > 2/- The code, even if it did check, would only check the last 3000 bytes
> >   in the file.  The section can occur anywhere in the last 3000
> >   CHARACTERS.
> > 3/- The code doesn't do a case-sensitive search for "elisp-shorthands".
> > 4/- The code doesn't check for "elisp-shorthands" being a complete
> >   symbol.
> > 5/- The code doesn't even check that "elisp-shorthands" is in a comment.

> > I would suggest that these errors be corrected.  I would also suggest
> > that the entire code and documentation for this new facility be
> > carefully reviewed by somebody who isn't the original author.

> Thanks, but why isn't this a full-blown bug report, submitted to
> debbugs?

The last sentence - I was hoping to recruit somebody (somebody more
enthusiastic about the feature than me) to do the reviewing, and this is
more likely on emacs-devel than debbugs.

But I'll resubmit it as a bug this evening.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Inadequate coding in hack-elisp-shorthands

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

On Fri, Oct 1, 2021, 18:04 Alan Mackenzie <acm@muc.de> wrote:

The last sentence - I was hoping to recruit somebody (somebody more
> enthusiastic about the feature than me) to do the reviewing, and this is
> more likely on emacs-devel than debbugs.
>

FYI this code was presented to all the head maintainers of Emacs, and
reviewed by Richard, as carefully as we could arrange, I suppose. I was
recently explicitly asked to rush it for Emacs 28.

But I'll fix these real bugs, which are completely secondary (but
nevertheless bugs).  Anyway, you might want to look at lread.c. That's
where the real juice of the feature lives and where any bugs would be more
interesting.

Thanks,
João

>

[-- Attachment #2: Type: text/html, Size: 1336 bytes --]

Re: Inadequate coding in hack-elisp-shorthands

[João Távora]

João Távora <joaotavora@gmail.com> writes:

> But I'll fix these real bugs, which are completely secondary (but
> nevertheless bugs).

Now done in emacs-28.  See bug#50946 for details.