Home
JAQForum Ver 20.06
Log In or Join  
Active Topics
Local Time 14:29 19 Apr 2024 Privacy Policy
Jump to

Notice. New forum software under development. It's going to miss a few functions and look a bit ugly for a while, but I'm working on it full time now as the old forum was too unstable. Couple days, all good. If you notice any issues, please contact me.

Forum Index : Microcontroller and PC projects : MMB4W: bug with maximum label length

Author Message
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 11:50pm 21 Jan 2023
Copy link to clipboard 
Print this post

Hi Peter,

Apologies in advance.

I believe the maximum label length should be 32 characters, it is/was with MMBasic for DOS (and MMB4L), so is it possible something went "wrong" when you switched to hash lookup of labels? in which case presumably CMM2 and PicoMite are affected too?

On Error Skip 1
Goto goto_max_length_label_3456789012
my_return:
Print Choice(Mm.ErrMsg$ = "", "OK", Mm.ErrMsg$)

On Error Skip 1
Goto goto_too_long_label_1234567890123
Print Choice(InStr(Mm.ErrMsg$, "Label too long"), "OK", Mm.ErrMsg$)

On Error Skip 1
Gosub gosub_max_length_label_456789012
Print Choice(Mm.ErrMsg$ = "", "OK", Mm.ErrMsg$)

On Error Skip 1
Gosub gosub_too_long_label_234567890123
Print Choice(InStr(Mm.ErrMsg$, "Label too long"), "OK", Mm.ErrMsg$)

On Error Skip 1
Restore restore_max_length_label_6789012
Print Choice(Mm.ErrMsg$ = "", "OK", Mm.ErrMsg$)

On Error Skip 1
Restore restore_too_long_label_4567890123
Print Choice(InStr(Mm.ErrMsg$, "Variable name too long"), "OK", Mm.ErrMsg$)

End

goto_max_length_label_3456789012:
 goto my_return

gosub_max_length_label_456789012:
 Return

restore_max_length_label_6789012:
 Data 0, 1, 2


Expected:
> run
OK
OK
OK
OK
OK
OK


Actual:
> run
Error in line 2: Cannot find label
OK
Error in line 11: Cannot find label
OK
Error in line 19: Cannot find label
OK


Best wishes,

Tom
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
twofingers
Guru

Joined: 02/06/2014
Location: Germany
Posts: 1133
Posted: 12:32am 22 Jan 2023
Copy link to clipboard 
Print this post

  thwill said  ... in which case presumably CMM2 and PicoMite are affected too?

Can confirm for Picomite. Output from Picomite:
Saved 730 bytes
Cannot find label
OK
Cannot find label
OK
Cannot find label
OK
 
TassyJim

Guru

Joined: 07/08/2011
Location: Australia
Posts: 5880
Posted: 01:53am 22 Jan 2023
Copy link to clipboard 
Print this post

My uneducated guess is, the hash table routines haven't allowed for the trailing zero byte.

In a lot of places in the source I see
MAXVARLEN + 1
but not in the hash table stuff.

Jim
VK7JH
MMedit   MMBasic Help
 
matherp
Guru

Joined: 11/12/2012
Location: United Kingdom
Posts: 8566
Posted: 08:15am 22 Jan 2023
Copy link to clipboard 
Print this post

Don't care - won't be fixing
 
Mixtel90

Guru

Joined: 05/10/2019
Location: United Kingdom
Posts: 5705
Posted: 08:41am 22 Jan 2023
Copy link to clipboard 
Print this post


Long labels are a pain to type anyway...



;)
Mick

Zilog Inside! nascom.info for Nascom & Gemini
Preliminary MMBasic docs & my PCB designs
 
CaptainBoing

Guru

Joined: 07/09/2016
Location: United Kingdom
Posts: 1985
Posted: 09:45am 22 Jan 2023
Copy link to clipboard 
Print this post

+1 +1

long labels (and variable names) need to be "within reason" and on the slower platforms there can be really good reasons to avoid them
Edited 2023-01-22 19:46 by CaptainBoing
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 11:31am 22 Jan 2023
Copy link to clipboard 
Print this post

  CaptainBoing said  +1 +1

long labels (and variable names) need to be "within reason" and on the slower platforms there can be really good reasons to avoid them


Very valid arguments, but contrawise when writing a sufficiently complex piece of software with many modules long identifiers are exceeding useful to make the code readable and avoid naming conflicts.

Anyway, not really what this was about, the "spec" says identifiers are 32 characters, I found a case where this was not true, Peter isn't going to fix it, neither am I at the moment (except for MMB4L), job "done".

Best wishes,

Tom
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 12:00pm 22 Jan 2023
Copy link to clipboard 
Print this post

OK, here is another unexpected 31 character limit, slightly more significant since it happens with functions rather than (obsolete) labels.

> list
Option Explicit On

On Error Skip
Dim i% = fun_with_max_length_name_6789012%()
? Choice(Mm.ErrMsg$ = "", "OK", "Caught: " + Mm.ErrMsg$)

Function fun_with_max_length_name_6789012%()
 fun_with_max_length_name_6789012% = 42
End Function

> run
Caught: Error in line 7: Variable name too long


I believe the problem is in MMBasic.c#skipvar():

const char *skipvar(const char *p, int noerror) {
   const char *pp;
   const char *tp;
   int i;
   int inquote = false;

   tp = p;
   // check the first char for a legal variable name
   skipspace(p);
   if(!isnamestart(*p)) return tp;

   do {
       p++;
   } while(isnamechar(*p));

   // check the terminating char.
   if(*p == '$' || *p == '%' || *p == '!') p++;

   if(p - tp > MAXVARLEN) {
       if(noerror) return p;
       error("Variable name too long");
   }


Note how the 1 character type extension is incorrectly (at least it doesn't seem to be counted in findvar() or stored in the name field of vartbl) being included in the variable name length check.

Best wishes,

Tom
Edited 2023-01-22 22:00 by thwill
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
matherp
Guru

Joined: 11/12/2012
Location: United Kingdom
Posts: 8566
Posted: 12:03pm 22 Jan 2023
Copy link to clipboard 
Print this post

Nobody uses names this long. I'll get the manual changed to say the maximum for labels and functions is 31
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 12:22pm 22 Jan 2023
Copy link to clipboard 
Print this post

  matherp said  Nobody uses names this long. I'll get the manual changed to say the maximum for labels and functions is 31


It depends on whether you use the extension, and it's actually 30 for function names in some cases because later on skipvar() counts the opening bracket when its determining the name length:

   pp = p; skipspace(pp); if(*pp == '(') p = pp;
   if(*p == '(') {
       // this is an array <--- also not necessarily true.

       p++;
       if(p - tp > MAXVARLEN) {
           if(noerror) return p;
           error("Variable name too long");
       }


Also it appears in the presence of a type extension, variable names may be limited to 31 chars too when the interpreter goes through skipvar().

All, I'd appreciate some advice here, I get the feeling I may not be making friends and influencing people. Do I keep these things to myself or do I keep bringing them up?

Best wishes,

Tom
Edited 2023-01-22 22:24 by thwill
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
matherp
Guru

Joined: 11/12/2012
Location: United Kingdom
Posts: 8566
Posted: 12:32pm 22 Jan 2023
Copy link to clipboard 
Print this post

By all means bring them up IFF like the GOSUB you provide solutions. But these extreme edge cases that no-one is going to find in the real world or if they do can work round without issue seem to me pointless for a hobby focused product.

Changing, for example, skipvar could have many unforeseen consequences that the fixing the "bug" can't justify
Edited 2023-01-22 22:34 by matherp
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 12:36pm 22 Jan 2023
Copy link to clipboard 
Print this post

  matherp said  By all means bring them up IFF like the GOSUB you provide solutions. But these extreme edge cases that no-one is going to find in the real world or if they do can work round without issue seem to me pointless for a hobby focused product.


Thanks.

  Quote  Changing, for example, skipvar could have many unforeseen consequences that the fixing the "bug" can't justify


This is because you don't have unit-testing, which is designed to catch those unforeseen consequences . As it happens skipvar() is only used in a handful of places, I'll work on a fixed version for you and then you can make your own choice.

Best wishes,

Tom
Edited 2023-01-22 22:37 by thwill
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
CaptainBoing

Guru

Joined: 07/09/2016
Location: United Kingdom
Posts: 1985
Posted: 12:46pm 22 Jan 2023
Copy link to clipboard 
Print this post

  thwill said  

...it appears in the presence of a type extension, variable names may be limited to 31 chars too ...


It does. I am presuming this section of source is ancient and so largely aplicable to most (all?) versions.

I noted this point here: "you can see this 32 character thing for yourself if you try to define a variable with 32 character name, then do the same with a type indicator. It breaks. Remove one character (so now it's 31 chars long) and you can specify the type."
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 12:59pm 22 Jan 2023
Copy link to clipboard 
Print this post

  CaptainBoing said  It does. I am presuming this section of source is ancient and so largely aplicable to most (all?) versions.


I suspect so.

  CaptainBoing said  I noted this point here:


Indeed you did. It looks like findvar(), at least in Peter's versions and IIRC "MMBasic for DOS" doesn't consider the extension when evaluating the variable name limit, but skipvar(), which is also used to skip function prototypes/calls does.

If I can fix it for MMB4L then I'll port the changes to MMB4W and Peter and Geoff can take it from there if they wish.

Best wishes,

Tom
Edited 2023-01-22 23:01 by thwill
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
JohnS
Guru

Joined: 18/11/2011
Location: United Kingdom
Posts: 3649
Posted: 12:53pm 23 Jan 2023
Copy link to clipboard 
Print this post

  thwill said  
  matherp said  Nobody uses names this long. I'll get the manual changed to say the maximum for labels and functions is 31


Also it appears in the presence of a type extension, variable names may be limited to 31 chars too when the interpreter goes through skipvar().

All, I'd appreciate some advice here, I get the feeling I may not be making friends and influencing people. Do I keep these things to myself or do I keep bringing them up?

Best wishes,

Tom

I'm happy you're finding them.

I'd be happy if the manual changed to say max 30 (to cover all the above).

I'm fairly sure I've never "needed" even 30 :)

John
 
JohnS
Guru

Joined: 18/11/2011
Location: United Kingdom
Posts: 3649
Posted: 12:56pm 23 Jan 2023
Copy link to clipboard 
Print this post

  thwill said  This is because you don't have unit-testing

It's great you're making the tests.

As you know, they're hard work, though, and time-consuming.

Please don't be put off.  I feel overall that MMBasic will get even better.

John
 
thwill

Guru

Joined: 16/09/2019
Location: United Kingdom
Posts: 3830
Posted: 01:21pm 23 Jan 2023
Copy link to clipboard 
Print this post

  JohnS said  It's great you're making the tests.


Thanks John.

  JohnS said  I'd be happy if the manual changed to say max 30 (to cover all the above).


My recommendation is to leave the manual as it is for the moment since only I (and The Captain on the older 'mites) seem to have noticed these edge-cases; 32 is a much nicer number than 30.

When I've finished my current iteration of MMB4L I'll cross-port some fixes to MMB4W (not completely trivial because MMB4L has considerably rewritten identifier handling which I don't intend to port) which Peter may or may not consider for his other ports.

Best wishes,

Tom
Game*Mite, CMM2 Welcome Tape, Creaky old text adventures
 
Mixtel90

Guru

Joined: 05/10/2019
Location: United Kingdom
Posts: 5705
Posted: 02:11pm 23 Jan 2023
Copy link to clipboard 
Print this post

  Quote  32 is a much nicer number than 30

42 is even nicer. Is anyone porting Deep Thought to the PicoMite?

(Grabs towel and copy of HHGTTG. Runs away...)
Mick

Zilog Inside! nascom.info for Nascom & Gemini
Preliminary MMBasic docs & my PCB designs
 
Print this page


To reply to this topic, you need to log in.

© JAQ Software 2024