You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/02/09 18:43:31 UTC
[GitHub] [arrow] rsaccoccio opened a new pull request #12380: ARROW-15626: GIOInputStream can return short reads
rsaccoccio opened a new pull request #12380:
URL: https://github.com/apache/arrow/pull/12380
If GIOInputStream is used with a file descriptor that can return short reads, it will too. This breaks the assumption by callers that the Read() will return all that it asked for.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12380: ARROW-15626: GIOInputStream can return short reads
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12380:
URL: https://github.com/apache/arrow/pull/12380#issuecomment-1034080356
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #12380: ARROW-15626: [GLib] GIOInputStream can return short reads
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #12380:
URL: https://github.com/apache/arrow/pull/12380#discussion_r803111744
##########
File path: c_glib/arrow-glib/input-stream.cpp
##########
@@ -833,17 +835,19 @@ namespace garrow {
std::lock_guard<std::mutex> guard(lock_);
GError *error = NULL;
- auto n_read_bytes = g_input_stream_read(input_stream_,
- buffer->mutable_data(),
- n_bytes,
- NULL,
- &error);
- if (n_read_bytes == -1) {
+ gsize n_read_bytes = 0;
+ g_input_stream_read_all(input_stream_,
+ buffer->mutable_data(),
+ n_bytes,
+ &n_read_bytes,
+ NULL,
+ &error);
+ if (error) {
return garrow_error_to_status(error,
arrow::StatusCode::IOError,
"[gio-input-stream][read][buffer]");
} else {
- if (n_read_bytes < n_bytes) {
+ if ((int64_t) n_read_bytes < n_bytes) {
Review comment:
```suggestion
if (n_read_bytes < static_cast<gsize>(n_bytes)) {
```
##########
File path: c_glib/arrow-glib/input-stream.cpp
##########
@@ -802,12 +802,14 @@ namespace garrow {
arrow::Result<int64_t> Read(int64_t n_bytes, void *out) override {
std::lock_guard<std::mutex> guard(lock_);
GError *error = NULL;
- auto n_read_bytes = g_input_stream_read(input_stream_,
- out,
- n_bytes,
- NULL,
- &error);
- if (n_read_bytes == -1) {
+ gsize n_read_bytes = 0;
+ g_input_stream_read_all(input_stream_,
+ out,
+ n_bytes,
+ &n_read_bytes,
+ NULL,
+ &error);
+ if (error) {
Review comment:
We should use the return value of `g_input_stream_read_all()` instead of `error` to detect error:
```suggestion
auto success = g_input_stream_read_all(input_stream_,
out,
n_bytes,
&n_read_bytes,
NULL,
&error);
if (success) {
return n_read_bytes;
} else {
return garrow_error_to_status(error,
arrow::StatusCode::IOError,
"[gio-input-stream][read]");
}
```
See also: https://docs.gtk.org/gio/method.InputStream.read_all.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org