TinyScheme seg faults due to the bad 'let' syntax. Some other seg faults due to bad let syntax were recently fixed which will make it easier to solve this bug.

The seg fault occurs due to 'caar(x)' in the second for loop under "case OP_LET2". 

Unless the fix for this is a major change, this should go into 2.4 as well.

This may be the wrong bug report for it, but Kevin didn't point to a bug report in his latest changes, so either he missed the reference or he failed to open a bug report for it.

Anyway, I said that unless the fix is a major change, it should go into 2.4. Your recent changes in Script-Fu are far beyond major. They break lots of scripts *again*. Even a large list of the scripts that we ship with GIMP need to be updated. This is unacceptable and definitely should be reconsidered.

I strongly suggest that you revert the changes that enforce R5RS syntax for 'let' in Script-Fu. We can't break people's scripts again, definitely not in the middle of the GIMP 2.4 series. But I would even advocate for not breaking them at all, not even for GIMP 2.6. It is IMO better to have Script-Fu crash under some rare circumstances than forcing our users to update their scripts which used to work just fine.

The other causes of a seg fault in Script-Fu due to bad let syntax was reported to the SourceForge bug tracking system for TinyScheme. I didn't bother duplicating that report in this bug tracking system.

The seg fault fixes were a minor change. It was several days later when I learned that it also enforced R5RS syntax. If I had used the proper syntax when updating the 95+ Script-Fu scripts for use with TinyScheme, there would not have been bad examples for people to copy when updating their own scripts and scripts would not have broken again with the recent fix in TinyScheme.

Sven and I have discussed this on #gimp and are reviewing some options as to how to deal with the problem. It would not be good to leave in something that can result in a seg fault that would kill Script-Fu since it would require a restart of GIMP to restore access to Script-Fu scripts.

Let's keep this bug focused on the reported crash due to bad 'let' syntax. I have opened a new report (bug #512105) for the incompatibility introduced by the recent changes to Script-Fu.

> (let x 5)
Error: Undefined variable 

This is what I get after inserting two lines in scheme.c after line 2786:

          if (is_symbol(car(sc->code))) {    /* named let */
               if (!is_pair(cadr(sc->code)))
                    Error_0(sc, "Undefined variable");
               for (x = cadr(sc->code), sc->args = sc->NIL;

In other words, at least for this particular case it fixes the crash.
Otherwise no guarantees given.
(I do not want to suggest that this would be a proper fix. If people tell me what kind of a fix is desired, I could try and make a patch.)

This looks like a pretty good fix, or at least a good workaround. Perhaps the error message could be improved a little. It would be nice if it said something "Error: Undefined variable x in let statement". Even better if it would mention the line where the error occurs, but that is handled in bug #497913 already.

The fix is correct but the error message is wrong. It is a case of bad syntax and not an undefined variable. There are two levels of parentheses missing around the "x 5" part of the let statement.

A more correct error message would be
Syntax error in let : missing parentheses

Can't you instead of nitpicking just commit a proper fix? This is blocking the 2.4.6 release and it would be really nice if it eventually be settled.

Created attachment 110971 [details] [review]
fix let syntax error crash

> It would be nice if it said something like
> "Error: Undefined variable x in let statement".
> Even better if it would mention the line

As far as I understand, this part of the gimp code is very close to, almost
identical to, the original tinyscheme code. It would be nice to add additional
features, but probably unwise to do that in gimp. Better to inherit the
improvements from upstream, so that it also remains easy to inherit the
bugfixes.
(I am assuming that an active upstream still exists.)

> The fix is correct but the error message is wrong.

Yes, indeed. In the meantime I checked the syntax of let in other lisp-like
languages and see that (let ((x 5)) x) works fine.
By the way, is there a more precise reference than just saying that tinyscheme
is lisp-like? Maybe tinyscheme is a tiny project, but GIMP should be important
enough to have its scripting language well-documented?

2008-05-16  Kevin Cozens  <kcozens@cvs.gnome.org>

        * plug-ins/script-fu/tinyscheme/scheme.c: Added extra checks to stop
        bad syntax in LET from causing a segmentation fault in Linux.
        See bug #508020.

gymp: The version of TinyScheme in GIMP uses Scheme (a variant of Lisp) and is close to the original. There are changes and bug fixes in the original which aren't in the GIMP version (yet). Scheme itself is well documented. If you want to discuss issues relating to TinyScheme, feel free to do so on the mailing lists. This is not the place for such discussions.

2008-05-22  Kevin Cozens  <kcozens@cvs.gnome.org>

        * plug-ins/script-fu/tinyscheme/scheme.c: Added extra check to stop
        "(let x 5)" syntax from causing a segfault in Linux. See bug #508020.
        Removed some excess whitespace.

The previous version of this fix was reverted as it affected the use of named let's. If this recent change has no unexpected side effects, it should be backported to the 2.4 GIMP.

Created attachment 111557 [details] [review]
patch that was committed to trunk

For the reference, here's the patch that Kevin committed to trunk.

To me this looks a bit too dangerous for a back-port to the gimp-2-4 branch. We should probably give it some more testing in trunk and perhaps merge it to the stable branch for the 2.4.7 release.

I tested it on GIMP 2.6.4, and there is no crash right now. The error message I get is

> (let x 5)
Error: Bad syntax of binding in let : 5 

this seems to be ok - no crash, and the error message is OK (in another scheme console, not in GIMP, I saw asimilar error message

let: bad syntax in: (let x 5)

I think this can be closed since as scen said in Comment #5,
> Let's keep this bug focused on the reported crash due to bad 'let' syntax.

The console doesn't crash now ecen when I try different mistakes in the let syntax, and one of the reasons for this is that GIMP 2.6 already forces R5RS syntax.

Bug should be closed

OK, closing as FIXED then.