Implement MojoGetBufferInformation(), part 3.

Implement MojoGetBufferInformation(), part 3.

This hooks up Core::GetBufferInformation() to all the thunks, etc.

R=smklein@chromium.org, vardhan@google.com
BUG=#501

Committed: https://chromium.googlesource.com/external/mojo/+/9beb4f7377e271d83038be0bdaf53154ed6bc607

[viettrungluu, 2016-03-09 00:51:04]

smklein -> NaCl stuff, vardhan -> everything else

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/nonsfi/irt_mojo_n...
File mojo/nacl/nonsfi/irt_mojo_nonsfi.cc (right):

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/nonsfi/irt_mojo_n...
mojo/nacl/nonsfi/irt_mojo_nonsfi.cc:19: nacl::MojoGetInitialHandle,
This had to be reordered to match the reordering of interface.py (I wonder if
this shouldn't be generated too. <shrug>)

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
File mojo/nacl/sfi/nacl_bindings/mojo_syscall.cc (right):

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
mojo/nacl/sfi/nacl_bindings/mojo_syscall.cc:51: if (num_params != 3) {
This enormous diff is nearly entirely due to reordering in interface.py.

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
mojo/nacl/sfi/nacl_bindings/mojo_syscall.cc:664: case 17: {
This case is the only new one (due to the corresponding addition to
interface.py). See also the addition to interface_dsl.py.

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
File mojo/nacl/sfi/nacl_bindings_generator/interface.py (right):

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
mojo/nacl/sfi/nacl_bindings_generator/interface.py:12: # This function is not
provided by the Mojo system APIs, but instead allows
I reordered these into a more sensible order, which affects the NaCl thunks
rather severely (and also the generated code).

But after this, more functions can be appended with small diffs resulting (in
generated code). (Whereas the previous pattern of prepending stuff always in big
diffs.)

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
mojo/nacl/sfi/nacl_bindings_generator/interface.py:116: f =
mojo.Func('MojoGetBufferInformation', 'MojoResult')
Other than reordering, this is the only change (addition) in this file.

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
File mojo/nacl/sfi/nacl_bindings_generator/interface_dsl.py (right):

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
mojo/nacl/sfi/nacl_bindings_generator/interface_dsl.py:122: # Out extensible
structs have an input size indicating the buffer size. On
This (and the resulting generated code) should be considered carefully. I think
it's right....

[viettrungluu, 2016-03-09 00:51:54]

Description was changed from

==========
Implement MojoGetBufferInformation(), part 3.

This hooks up Core::GetBufferInformation() to all the thunks, etc.

R=vardhan@google.com, smklein@chromium.org
==========

to

==========
Implement MojoGetBufferInformation(), part 3.

This hooks up Core::GetBufferInformation() to all the thunks, etc.

R=vardhan@google.com, smklein@chromium.org
BUG=#501
==========

[viettrungluu, 2016-03-09 21:45:54]

ping?

[Sean Klein, 2016-03-09 22:18:22]

LGTM w.r.t. NaCl bits. Thanks for annotating where you relocated unchanged code.

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/nonsfi/irt_mojo_n...
File mojo/nacl/nonsfi/irt_mojo_nonsfi.cc (right):

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/nonsfi/irt_mojo_n...
mojo/nacl/nonsfi/irt_mojo_nonsfi.cc:19: nacl::MojoGetInitialHandle,
On 2016/03/09 00:51:04, viettrungluu wrote:
> This had to be reordered to match the reordering of interface.py (I wonder if
> this shouldn't be generated too. <shrug>)

The Non-SFI and SFI implementations may not always have feature parity, so I
think doing Non-SFI changes like this manually is fine.

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
File mojo/nacl/sfi/nacl_bindings_generator/interface_dsl.py (right):

https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
mojo/nacl/sfi/nacl_bindings_generator/interface_dsl.py:122: # Out extensible
structs have an input size indicating the buffer size. On
On 2016/03/09 00:51:04, viettrungluu wrote:
> This (and the resulting generated code) should be considered carefully. I
think
> it's right....

Seems correct to me. Slightly confusing that it is identified as "not an array"
here, but uses the "ConvertArray" function later (I understand why, but it's
still a little weird).

[vardhan, 2016-03-09 23:43:13]

lgtm

[viettrungluu, 2016-03-09 23:58:19]

Description was changed from

==========
Implement MojoGetBufferInformation(), part 3.

This hooks up Core::GetBufferInformation() to all the thunks, etc.

R=vardhan@google.com, smklein@chromium.org
BUG=#501
==========

to

==========
Implement MojoGetBufferInformation(), part 3.

This hooks up Core::GetBufferInformation() to all the thunks, etc.

R=smklein@chromium.org, vardhan@google.com
BUG=#501

Committed:
https://chromium.googlesource.com/external/mojo/+/9beb4f7377e271d83038be0bdaf...
==========

[viettrungluu, 2016-03-09 23:58:22]

Committed patchset #1 (id:1) manually as
9beb4f7377e271d83038be0bdaf53154ed6bc607 (presubmit successful).

[viettrungluu, 2016-03-09 23:59:53]

Thanks.

On 2016/03/09 22:18:22, Sean Klein wrote:
> LGTM w.r.t. NaCl bits. Thanks for annotating where you relocated unchanged
code.
> 
>
https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/nonsfi/irt_mojo_n...
> File mojo/nacl/nonsfi/irt_mojo_nonsfi.cc (right):
> 
>
https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/nonsfi/irt_mojo_n...
> mojo/nacl/nonsfi/irt_mojo_nonsfi.cc:19: nacl::MojoGetInitialHandle,
> On 2016/03/09 00:51:04, viettrungluu wrote:
> > This had to be reordered to match the reordering of interface.py (I wonder
if
> > this shouldn't be generated too. <shrug>)
> 
> The Non-SFI and SFI implementations may not always have feature parity, so I
> think doing Non-SFI changes like this manually is fine.

That's a good point.

> 
>
https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
> File mojo/nacl/sfi/nacl_bindings_generator/interface_dsl.py (right):
> 
>
https://codereview.chromium.org/1778753002/diff/1/mojo/nacl/sfi/nacl_bindings...
> mojo/nacl/sfi/nacl_bindings_generator/interface_dsl.py:122: # Out extensible
> structs have an input size indicating the buffer size. On
> On 2016/03/09 00:51:04, viettrungluu wrote:
> > This (and the resulting generated code) should be considered carefully. I
> think
> > it's right....
> 
> Seems correct to me. Slightly confusing that it is identified as "not an
array"
> here, but uses the "ConvertArray" function later (I understand why, but it's
> still a little weird).

Maybe it should be renamed to "ConvertBuffer", but I left it as-is for now.