Fix external snapshot reading

Fix external snapshot reading by removing an assumption that strings in the snapshot file are null-terminated.

R=vogelheim@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=23416

[baixo, 2014-08-22 10:05:12]

Please take a look.

[rmcilroy, 2014-08-22 10:17:20]

+vogelheim: Could you take a look please?  The external snapshot code was
crashing for Andre before he added this fix.

https://codereview.chromium.org/496253003/diff/1/tools/js2c.py
File tools/js2c.py (right):

https://codereview.chromium.org/496253003/diff/1/tools/js2c.py#newcode513
tools/js2c.py:513: value += '\0'
Add a comment stating why this is necessary.

[baixo, 2014-08-22 10:46:59]

On 2014/08/22 10:17:20, rmcilroy wrote:
> +vogelheim: Could you take a look please?  The external snapshot code was
> crashing for Andre before he added this fix.
> 
> https://codereview.chromium.org/496253003/diff/1/tools/js2c.py
> File tools/js2c.py (right):
> 
> https://codereview.chromium.org/496253003/diff/1/tools/js2c.py#newcode513
> tools/js2c.py:513: value += '\0'
> Add a comment stating why this is necessary.

Comment added.

[rmcilroy, 2014-08-22 13:55:18]

On 2014/08/22 10:46:59, baixo wrote:
> On 2014/08/22 10:17:20, rmcilroy wrote:
> > +vogelheim: Could you take a look please?  The external snapshot code was
> > crashing for Andre before he added this fix.
> > 
> > https://codereview.chromium.org/496253003/diff/1/tools/js2c.py
> > File tools/js2c.py (right):
> > 
> > https://codereview.chromium.org/496253003/diff/1/tools/js2c.py#newcode513
> > tools/js2c.py:513: value += '\0'
> > Add a comment stating why this is necessary.
> 
> Comment added.

Looks good to me, I'll leave vogelheim to approve.

[vogelheim, 2014-08-26 12:43:13]

Sorry for the late answer; I overlooked this. Please feel free to ping me if you
don't get an answer within 1 working day.

https://codereview.chromium.org/496253003/diff/20001/tools/js2c.py
File tools/js2c.py (right):

https://codereview.chromium.org/496253003/diff/20001/tools/js2c.py#newcode514
tools/js2c.py:514: value += '\0'
I really don't get this. Could you elaborate on what the problem is?

The reason why the string is length-prefixed is so that we don't have to rely on
string termination. If someone expects to strcmp the result of this, that would
indicate a problem at that end...

[baixo, 2014-08-26 15:36:26]

Changed to strncmp

https://codereview.chromium.org/496253003/diff/20001/tools/js2c.py
File tools/js2c.py (right):

https://codereview.chromium.org/496253003/diff/20001/tools/js2c.py#newcode514
tools/js2c.py:514: value += '\0'
You're right. Just changed the strcmp side.

On 2014/08/26 12:43:12, vogelheim wrote:
> I really don't get this. Could you elaborate on what the problem is?
> 
> The reason why the string is length-prefixed is so that we don't have to rely
on
> string termination. If someone expects to strcmp the result of this, that
would
> indicate a problem at that end...

[vogelheim, 2014-08-26 16:34:21]

lgtm

Thanks for fixing this!

[baixo, 2014-08-26 16:46:07]

The CQ bit was checked by baixo@chromium.org

[baixo, 2014-08-26 16:46:07]

The CQ bit was unchecked by baixo@chromium.org

[baixo, 2014-08-26 16:46:29]

The CQ bit was checked by baixo@chromium.org

[baixo, 2014-08-26 16:46:35]

The CQ bit was unchecked by baixo@chromium.org

[rmcilroy, 2014-08-26 16:57:04]

Committed patchset #3 manually as 23416 (presubmit successful).