jackxiao / jslibs

Automatically exported from code.google.com/p/jslibs
0 stars 0 forks source link

enhance J_S_ASSERT, J_CHK, ... #74

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
1/ when one of these 'tools' failed, there is no way to cleanup things
before the return JS_FALSE.
2/ if these 'tools' are used in a non-js-api-like function (no JSBool
return), we cannot manage the right return value.
3/ these 'tools' cannot be used in  returnless functions.
Today, we only "return JS_FALSE;"

Original issue reported on code.google.com by sou...@gmail.com on 19 Nov 2008 at 7:39

GoogleCodeExporter commented 9 years ago
fix:
each J_S_ASSERT, J_CHK, ... macro that fails go to a 'bad' label, and then 
returns
JS_FALSE;

Original comment by sou...@gmail.com on 19 Nov 2008 at 10:33

GoogleCodeExporter commented 9 years ago
The functions with the void return type can still be changed to return JSBool 
to 
make the macros useful.

IMO, since there is not so many instances of these macros, one option would be 
to 
unfold them ;)

Another option would be to pass the return value as additional macro parameter.

Original comment by goo...@ziak.com on 21 Nov 2008 at 5:54

GoogleCodeExporter commented 9 years ago
> The functions with the void return type can still be changed to return JSBool 
to 
> make the macros useful.

In some cases, when the underlying API define the prototype of a callback 
function
for example, there is no way to do this.
Furthermore the prototype of a function is an important choice and cannot be 
changed
only because some macros cannot match with this prototype.

> IMO, since there is not so many instances of these macros, one option would 
be to 
> unfold them ;)

There are exactly 3432 instances of these macros :) (svn -R ls | xargs grep -H 
XXX |
wc -l)

> Another option would be to pass the return value as additional macro 
parameter.

Of course, this was one of my options but in cases like these:

...
bad:
    if ( errorBuffer.buffer != NULL )
        free(errorBuffer.buffer);
    if ( cx )
        DestroyHost(cx);
    return 0;
}

it could be a little bit unreadable to put this as argument of a macros.

Note that the aim of these macros is to simplify the readability of the jslibs 
code.
But I admit that these macros are not very well documented.

Original comment by sou...@gmail.com on 21 Nov 2008 at 5:56

GoogleCodeExporter commented 9 years ago

Original comment by sou...@gmail.com on 27 Nov 2008 at 9:44