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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
