`with-X` forms panic if params is not a list

[dragoncoder047]

9043> (with-i2c foo)
Guru Meditation Error: Core  1 panic'ed (LoadProhibited). Exception was unhandled.

fix:

object* sp_withi2c (object* args, object* env) {
    object* params = first(args);
    if (params == NULL) error2(nostream);
+   if (atom(params)) error(notalist, params);
    object* var = first(params);
    int address = checkinteger(eval(second(params), env));
    params = cddr(params);

(and repeat similar all with-X forms!!)

[dragoncoder047]

Also in for-millis, same sort of bug but less obvious fix

object* sp_formillis (object* args, object* env) {
    if (args == NULL) error2(noargument);
    object* param = first(args);
    unsigned long start = millis();
    unsigned long now, total = 0;
-   if (param != NULL) total = checkinteger(eval(first(param), env));
+   if (param != NULL) {
+       if (atom(param)) error(notalist, param);
+       total = checkinteger(eval(first(param), env));
+   }
    eval(tf_progn(cdr(args),env), env);

[technoblogy]

Thanks. Will put this in next release.

[technoblogy]

It's a bit more complicated; it should also object to this:

(for-millis (500 s) (print "Go"))

Thinking about a general solution to this...

[dragoncoder047]

I'm thinking as a general solution, on special forms like this, you could have a function int checkparams(object* params, int min_length, int max_length) you would call it like this:

object* params = first(args);
int len = checkparams(params, 1, 1); // For (for-millis)
// do stuff

[technoblogy]

OK, here's a general solution:

/*
  checkarguments - checks the arguments list in a special form such as with-xxx,
  dolist, dotimes, for-millis, etc, and returns it.
*/
object *checkarguments (object *args, int min, int max) {
  if (args == NULL) error2(noargument);
  args = first(args);
  if (!listp(args)) error(notalist, args);
  int length = 0;
  object *list = args;
  while (list != NULL) {
    length++;
    if (length > max) error(toomanyargs, args);
    list = cdr(list);
  }
  if (length < min) error(toofewargs, args);
  return args;
}

It replaces the hand-crafted tests in each of those special forms; for example:

object *sp_formillis (object *args, object *env) {
  object *param = checkarguments(args, 0, 1);
  unsigned long start = millis();
  unsigned long now, total = 0;
  if (param != NULL) total = checkinteger(eval(first(param), env));
  eval(tf_progn(cdr(args),env), env);
  do {
    now = millis() - start;
    testescape();
  } while (now < total);
  if (now <= INT_MAX) return number(now);
  return nil;
}

[technoblogy]

I was writing that as you sent your comment! Great minds etc ...

[dragoncoder047]

I was thinking of something simpler:

/*
  checkarguments - checks the parameters list in a special form such as with-xxx,
  dolist, dotimes, for-millis, etc, and returns it.
*/
object *checkarguments (object *args, int min, int max) {
    if (args == NULL) error2(noargument);
    args = first(args);
    if (!listp(args)) error(notalist, args);
-   int length = 0;
-   object *list = args;
-   while (list != NULL) {
-       length++;
-       if (length > max) error(toomanyargs, args);
-       list = cdr(list);
-   }
+   int length = listlength(args);
+   if (length < min) error(toomanyargs, args);
    if (length > max) error(toofewargs, args);
    return args;
}

...you already have listlength(), why not use it?

[technoblogy]

Yes, that's neater. I wanted to avoid checking the length if we'd already exceeded max, but perhaps that's unnecessary.

[dragoncoder047]

but perhaps that's unnecessary.

You're going to throw an error anyway, so I would agree.

[technoblogy]

These should all now be fixed in Release 4.4b - thanks for reporting them!