Mojo: DataPipe: Write some docs and rationalize some return values.

Mojo: DataPipe: Write some docs and rationalize some return values.

In "all or none" mode, return "out of range" if the required amount of
space/data isn't available, unless the other end isn't open (in which
case return "failed precondition"). Don't return "should wait", since
you can't wait for a specified amount of space/data to be available.
(Only return "should wait" when waiting would actually solve your
problem.)

Note: Still have to write docs for the two-phase DataPipe APIs (but I
want to fully test/hash out their semantics first).

R=darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243327

[darin (slow to review), 2014-01-07 04:28:47]

LGTM

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core.h
File mojo/public/system/core.h (right):

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:448: // will have an element size of one byte and have
some system-dependent
it might help to know this system dependent capacity as it can impact the
behavior of writes when MOJO_WRITE_DATA_FLAG_ALL_OR_NONE or is specified.
however, we might want to vary it depending on the device. hmm...

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:478: // data, starting with the earliest written data.
However, even with "may
nit: "earliest written data [that has not yet been consumed]" ?

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:480: // |MOJO_WRITE_DATA_FLAG_ALL_OR_NONE| is not
set), this write will the amount
nit: "write will the amount" -> "will write the [maximum] amount" ?

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:527: // May also be used to discard data or query the
amount of data available.
It is not clear to me why we have MOJO_READ_DATA_FLAG_QUERY here, but not in the
case of message pipes. For a message pipe, to find out the size of the next
message, you just try to read and it fails if your buffer is not big enough. Why
wouldn't we just say that you can set *num_bytes to 0 on input to learn how many
bytes are available to read from the a single call to MojoReadData?

[viettrungluu, 2014-01-07 05:45:25]

Thanks.

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core.h
File mojo/public/system/core.h (right):

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:448: // will have an element size of one byte and have
some system-dependent
On 2014/01/07 04:28:47, darin wrote:
> it might help to know this system dependent capacity as it can impact the
> behavior of writes when MOJO_WRITE_DATA_FLAG_ALL_OR_NONE or is specified.
> however, we might want to vary it depending on the device. hmm...

I want to add some ability to query information about a handle, including things
like this. I haven't figured out the exact API yet though.

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:478: // data, starting with the earliest written data.
However, even with "may
On 2014/01/07 04:28:47, darin wrote:
> nit: "earliest written data [that has not yet been consumed]" ?

Done.

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:480: // |MOJO_WRITE_DATA_FLAG_ALL_OR_NONE| is not
set), this write will the amount
On 2014/01/07 04:28:47, darin wrote:
> nit: "write will the amount" -> "will write the [maximum] amount" ?

Done.

https://codereview.chromium.org/118873004/diff/50001/mojo/public/system/core....
mojo/public/system/core.h:527: // May also be used to discard data or query the
amount of data available.
On 2014/01/07 04:28:47, darin wrote:
> It is not clear to me why we have MOJO_READ_DATA_FLAG_QUERY here, but not in
the
> case of message pipes. For a message pipe, to find out the size of the next
> message, you just try to read and it fails if your buffer is not big enough.
Why
> wouldn't we just say that you can set *num_bytes to 0 on input to learn how
many
> bytes are available to read from the a single call to MojoReadData?

The situation isn't quite the same. For message pipes, reads always read a
single message, and tell you the size of the message. That you can set
*num_bytes to 0 and get the message size is a side effect of this. (Another
reasonable pattern for message pipes is to try to read into a
fixed/preallocated/stack-allocated buffer, and only heap-allocate if the next
message is too big.)

Data pipes are unframed and are happy to read any amount of data, so we don't
have this side effect. I didn't want to overload the meaning of *num_bytes in
this way....

(Maybe there's an argument for an explicit "query" flag for message pipes.)

[darin (slow to review), 2014-01-07 05:47:44]

On Mon, Jan 6, 2014 at 9:45 PM, <viettrungluu@chromium.org> wrote:

> Thanks.
>
>
>
> https://codereview.chromium.org/118873004/diff/50001/mojo/
> public/system/core.h
> File mojo/public/system/core.h (right):
>
> https://codereview.chromium.org/118873004/diff/50001/mojo/
> public/system/core.h#newcode448
> mojo/public/system/core.h:448: // will have an element size of one byte
> and have some system-dependent
> On 2014/01/07 04:28:47, darin wrote:
>
>> it might help to know this system dependent capacity as it can impact
>>
> the
>
>> behavior of writes when MOJO_WRITE_DATA_FLAG_ALL_OR_NONE or is
>>
> specified.
>
>> however, we might want to vary it depending on the device. hmm...
>>
>
> I want to add some ability to query information about a handle,
> including things like this. I haven't figured out the exact API yet
> though.


OK, sounds good.



>
>
> https://codereview.chromium.org/118873004/diff/50001/mojo/
> public/system/core.h#newcode478
> mojo/public/system/core.h:478: // data, starting with the earliest
> written data. However, even with "may
> On 2014/01/07 04:28:47, darin wrote:
>
>> nit: "earliest written data [that has not yet been consumed]" ?
>>
>
> Done.
>
>
> https://codereview.chromium.org/118873004/diff/50001/mojo/
> public/system/core.h#newcode480
> mojo/public/system/core.h:480: // |MOJO_WRITE_DATA_FLAG_ALL_OR_NONE| is
> not set), this write will the amount
> On 2014/01/07 04:28:47, darin wrote:
>
>> nit: "write will the amount" -> "will write the [maximum] amount" ?
>>
>
> Done.
>
>
> https://codereview.chromium.org/118873004/diff/50001/mojo/
> public/system/core.h#newcode527
> mojo/public/system/core.h:527: // May also be used to discard data or
> query the amount of data available.
> On 2014/01/07 04:28:47, darin wrote:
>
>> It is not clear to me why we have MOJO_READ_DATA_FLAG_QUERY here, but
>>
> not in the
>
>> case of message pipes. For a message pipe, to find out the size of the
>>
> next
>
>> message, you just try to read and it fails if your buffer is not big
>>
> enough. Why
>
>> wouldn't we just say that you can set *num_bytes to 0 on input to
>>
> learn how many
>
>> bytes are available to read from the a single call to MojoReadData?
>>
>
> The situation isn't quite the same. For message pipes, reads always read
> a single message, and tell you the size of the message. That you can set
> *num_bytes to 0 and get the message size is a side effect of this.
> (Another reasonable pattern for message pipes is to try to read into a
> fixed/preallocated/stack-allocated buffer, and only heap-allocate if the
> next message is too big.)
>
> Data pipes are unframed and are happy to read any amount of data, so we
> don't have this side effect. I didn't want to overload the meaning of
> *num_bytes in this way....
>
>
Makes sense.



> (Maybe there's an argument for an explicit "query" flag for message
> pipes.)
>

^^^ Yeah, I was wondering about that too.



>
> https://codereview.chromium.org/118873004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-01-07 09:57:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/118873004/14...

[commit-bot: I haz the power, 2014-01-07 10:55:12]

Retried try job too often on mac_rel for step(s) app_list_unittests,
cacheinvalidation_unittests, cc_unittests, check_deps, check_deps2git,
chromedriver_unittests, components_unittests, content_browsertests,
content_unittests, crypto_unittests, google_apis_unittests, gpu_unittests,
ipc_tests, jingle_unittests, media_unittests, message_center_unittests,
nacl_integration, ppapi_unittests, printing_unittests, remoting_unittests,
sql_unittests, sync_integration_tests, sync_unit_tests, telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[viettrungluu, 2014-01-07 17:50:13]

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