[1/2] jscript: Return NULL if jsstr_substr fails.

[Jacek Caban]

On 07.09.2016 08:45, Sebastian Lackner wrote:
> 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.

Thanks.

> 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?

I'm not opposed to this function if really needed, but in cases where we
can avoid it, I'd much rather do that. In case of Date_toTimeStringi
avoiding it seems easy. There is constant and relatively small maximum
buffer size and the code path is not performance critical, so I'd say
that using a buffer on stack to prepare the result and then pass it to
jsstr_alloc would be nicer IMHO.

Thanks,
Jacek