On 06.09.2016 15:26, Sebastian Lackner wrote:
On 06.09.2016 12:27, Jacek Caban wrote:
Hi Sebastian,
I took a deeper look at this. It seems that we have at least two more jsstr_alloc_buf callers to fix (jsstr_concat and JSGlobal_decodeURIComponent). All those seem to be mistakes caused by wrong choice of function signature, requiring not intuitive error handling. I'd prefer to change the root of those problems, making the function: jsstr_t *jsstr_alloc_buf(unsigned,WCHAR**) This should avoid similar mistakes in the future.
This will be much larger patch than you originally meant, so please let me know if you'd like to prepare a patch or should I take care of that.
Thanks, Jacek
Good catch. Yes, the current function signature makes it very easy to do things wrong. If you want me to prepare an updated patch I can do that, but I also don't mind if you want to prepare a patch yourself.
Best regards, Sebastian
I've just sent a patch, but feel free to ignore it if you have already been working on your own version.
By the way, there is another aspect which makes jsstr_alloc_buf very hard to use, which is that we have to know the exact buffer size in advance. As a result there are also places where it is done wrong, for example Date_toTimeString which passes the maximum required buffer size, but does not update ->length_flags afterwards. Are you fine with adding something like
jsstr_shrink_buf(jsstr_t *) or jsstr_shrink_buf(jsstr_t *, unsigned)
to fix this, or do we really want to fix the length calculation instead?
Regards, Sebastian