WARNING: jsvGetString overflowed

[espruino-discuss]

Posted at 2015-09-16 by DrAzzy

What does this mean, and why am I getting it?

That error occurs when I call this function, getMirrorString().

function getMirrorString() {
  var s=systemStatus;
  var tstr="?door_up="+s.door_upstairs+"&door_down="+s.door_downstairs+"&fridge="+s.fridge;
  tstr+=(s.light.clear>=0?"&clear="+s.light.clear+"&red="+s.light.red+"&green="+s.light.green+"&blue="+s.light.blue:"");
  tstr+=(s.pressure>0?"&pressure="+s.pressure:"")+"&LastMove="+safeGetTime()-s.lastMotion;
}

function safeGetTime(){
  if (clk) {
    return clk.getDate().getTime();
  } else {
    return -1;
  }
}

  systemStatus={
    light:{
      clear:-1,
      red:-1,
      green:-1,
      blue:-1
    },
    temperature:-1,
    pressure: -1,
    RFDevs:[0,0,0],
    door_upstairs:0,
    door_downstairs:0,
    fridge:0,
    lastMotion:0,
    fargo:new Uint8Array(8)
  };

[espruino-discuss]

Posted at 2015-09-16 by alexanderbrevig

The error is generated here: https://github.com/espruino/Espruino/blob/2485bac1862c111a66acf077803bb49c3bc8a15c/src/jsvar.c#L870

Seems like it tries to dump a jsVar into a string buffer but fails as it iterates beyond the reported length of the string buffer.

EDIT: I do not get the error when running locally, though I've changed safeGetTime to simply return a number or an object literal. Both works.

[gfwilliams]

Posted at 2015-09-16 by @gfwilliams

I don't get the error either (I just get clk not defined on new builds). Any chance you could try and get a bit of code that will produce the error alone? It could be to do with what's in clk.

The error happens when Espruino tries to get the value of something as a C string and it's too big for the buffer (usually 32 chars or so) - it actually does that pretty rarely, but I think you might be able to get it to happen sometimes with a super-long field name.

If you could get a self-contained bit of code that reproduces the error, hopefully I can run it in a debugger here, track down what calls jsvGetString, and try and fix it.

[espruino-discuss]

Posted at 2015-09-16 by DrAzzy

clk comes from here, which is called during initialization - it's just a clock object set to the current time.

I was getting this behavior with the latest cutting edge build - forgot to mention this in original post.

function getDate() {
  console.log("getDate called");
  var date="";
  require("http").get(dateurl, function(res) {
    res.on('data',function (data) {date+=data;});
    res.on('close',function() {console.log("onClose called");clk=new Clock(date);});
  });
  //delete getDate;
}

The dateurl is a url of a page that returns a string containing the current date (modified to correct for the time zone issue), and I believe that is working correctly, since I have the same code using the same URL on my desk light, where it gets the time successfully (Espruino Board 1v3 with v80 (ie, post v80 release, new numbering system) bigram).

The reason I use safeGetTime() is that if the initial request to get the date fails, I don't want every call to getMirrorString() to fail, since it could still be recording information, and if it gets back -1 for the current time, and then subtracts a bunch from it, it's still okay.

The code being run is https://github.com/SpenceKonde/AzzyProjects/edit/master/VoiceController.js - except I forgot to click sync last night so the version in github is from before I fixed the syntax errors. x_x

[espruino-discuss]

Posted at 2015-09-17 by DrAzzy

Clock = require("clock").Clock;

clk=new Clock("Thu, 17 Sep 2015 23:35:52");

  systemStatus={
    light:{
      clear:-1,
      red:-1,
      green:-1,
      blue:-1
    },
    temperature:-1,
    pressure: -1,
    RFDevs:[0,0,0],
    door_upstairs:0,
    door_downstairs:0,
    fridge:0,
    lastMotion:0,
    fargo:new Uint8Array(8)
  };

function getMirrorString() {
  var s=systemStatus;
  var tstr="?door_up="+s.door_upstairs+"&door_down="+s.door_downstairs+"&fridge="+s.fridge;
  tstr+=(s.light.clear>=0?"&clear="+s.light.clear+"&red="+s.light.red+"&green="+s.light.green+"&blue="+s.light.blue:"");
  tstr+=(s.pressure>0?"&pressure="+s.pressure:"")+"&LastMove="+safeGetTime()-s.lastMotion;
}

function safeGetTime(){
  if (clk) {
    return clk.getDate().getTime();
  } else {
    return -1;
  }
}

 _____                 _
|   __|___ ___ ___ _ _|_|___ ___
|   __|_ -| . |  _| | | |   | . |
|_____|___|  _|_| |___|_|_|_|___|
          |_| http://espruino.com
 1v80.334 Copyright 2015 G.Williams
>echo(0);
=undefined
>getMirrorString()
WARNING: jsvGetString overflowed
=undefined
> 

[gfwilliams]

Posted at 2015-09-17 by @gfwilliams

Perfect - thanks!

I just tracked this down, and it's: "&LastMove="+safeGetTime()-s.lastMotion.

This is like "Foo"+2-1 - the precedence of operators means that you're actually doing:

"Foo"+2-1
"Foo2"-1
NaN

What's happening is the - tries to convert Foo2 to a float, but the string buffer it reads it into is only sized for a floating point value - and "&LastMove=1442532952000.2280273" is bigger than it's expecting so it complains.

Still shouldn't come up with the warning though - I'll see what I can do about it.

[espruino-discuss]

Posted at 2015-09-17 by @allObjects

...would not waste much of breath on that (other than some comment in the doc)... operator overloading was always bad and gets worse with auto conversion in expressions with mixed types... Because I'm aware at design/coding time what is expected to be a number and what a string, I use parenthesis... (to be gentle - forcefully clear - to the underlaying system). I had to use that also when using shift operations with calculated shift values in compound expressions. The Foo example makes it perfetly clear...: the first + is obviously intended as a string concatenation, and the second an add which has precedence. When I need a string as a result and the concatenations starts with a number, I force string-precedence with "" + number1 + "string2" + (number2 + number3) + .... Alternative (on Browser) I like also [number1, "string2", number2 + number3, ...].join("");. I though do not know whether the alternatve creates a havoc for Espruino or some oder undesirable side effects.

[gfwilliams]

Posted at 2015-09-17 by @gfwilliams

Hmm, actually it's really difficult to do nicely. For instance:

parseInt("00000000000000000000000000000000000000000000000000000000000000000000043")

To parse it properly I'd have to actually have code to parse the number directly from a JsVar. Not sure what to do about that really - it could return NaN I guess, or maybe parsing from JsVar is the way to go.

[gfwilliams]

Posted at 2015-09-17 by @gfwilliams

Ok, so I partially fixed this. I upped the buffer size to 64 from 32, and if it does run out of space it gives a nicer error message like 'String too big to convert to float'

[espruino-discuss]

Posted at 2015-09-17 by alexanderbrevig

I did actually try that part specifically and when I tried to make it break. It simply added NaN to the end of the string. "test" + 2 - 1 => "test2NaN". But of course I never tried returning something long from my modified safeGetTime. I'm such a bad tester :(

[gfwilliams]

Posted at 2015-09-17 by @gfwilliams

I'm such a bad tester :(

Me too. Still, don't feel bad. Google's testers missed that too

[espruino-discuss]

Posted at 2015-09-17 by DrAzzy

Aaah. Of course - no wonder it didn't work, I forgot a pair of parenthesis to do the float math before adding it onto the string.

I wouldn't expect it to work, now that I see what I did. I think a better error message, like what Gordon implemented above, is all that's needed. "jsvGetString overflow" is a very unfriendly error (also - shouldn't this raise an exception? as opposed to returning undefined and printing a warning to console) - had I seen "String too big to convert to float" i'd have figured it out pretty quickly.

[gfwilliams]

Posted at 2015-09-18 by @gfwilliams

Good point. I'll swap it to an exception - at least then you can see where it is.