Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] IO: Style enhancement for io::CompressedInputStream #41116

Closed
mapleFU opened this issue Apr 10, 2024 · 1 comment
Closed

[C++] IO: Style enhancement for io::CompressedInputStream #41116

mapleFU opened this issue Apr 10, 2024 · 1 comment
Assignees
@mapleFU
Labels
Component: C++ Type: enhancement
Milestone
17.0.0

Comments

@mapleFU
Copy link
Member

mapleFU commented Apr 10, 2024

Describe the enhancement requested

Previously, #39807 has changed decompressed_ and compressed_ behavior. During I re-check this part of the code, I found the boundary check code is ugly. We may need to enhance this.

Component(s)

C++
@github-actions github-actions bot mentioned this issue Apr 10, 2024
Merged
mapleFU added a commit that referenced this issue Apr 15, 2024
@mapleFU @pitrou
GH-41116: [C++] IO: enhance boundary checking in CompressedInputStream (
271c878 
#41117)

### Rationale for this change

Enhance the boundary checking code style in `io::CompressedInputStream`.

### What changes are included in this PR?

* Add `compressed_buffer_available` and `decompressed_buffer_available` in the class, and uses them for checking the boundary
* Change `Status(bool*)` to `Result<bool>`

### Are these changes tested?

Already has testing. I don't know how to hacking into internal

### Are there any user-facing changes?

No

* GitHub Issue: #41116

Lead-authored-by: mwish <[email protected]>
Co-authored-by: mwish <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: mwish <[email protected]>
@mapleFU mapleFU added this to the 17.0.0 milestone Apr 15, 2024
@mapleFU
Copy link
Member Author

mapleFU commented Apr 15, 2024

Issue resolved by pull request 41117
#41117

@mapleFU mapleFU closed this as completed Apr 15, 2024
tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
@mapleFU @pitrou @tolleybot
apacheGH-41116: [C++] IO: enhance boundary checking in CompressedInpu…
9e58ac3 
…tStream (apache#41117)

### Rationale for this change

Enhance the boundary checking code style in `io::CompressedInputStream`.

### What changes are included in this PR?

* Add `compressed_buffer_available` and `decompressed_buffer_available` in the class, and uses them for checking the boundary
* Change `Status(bool*)` to `Result<bool>`

### Are these changes tested?

Already has testing. I don't know how to hacking into internal

### Are there any user-facing changes?

No

* GitHub Issue: apache#41116

Lead-authored-by: mwish <[email protected]>
Co-authored-by: mwish <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: mwish <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this issue May 4, 2024
@mapleFU @pitrou @tolleybot
apacheGH-41116: [C++] IO: enhance boundary checking in CompressedInpu…
52a7fe7 
…tStream (apache#41117)

### Rationale for this change

Enhance the boundary checking code style in `io::CompressedInputStream`.

### What changes are included in this PR?

* Add `compressed_buffer_available` and `decompressed_buffer_available` in the class, and uses them for checking the boundary
* Change `Status(bool*)` to `Result<bool>`

### Are these changes tested?

Already has testing. I don't know how to hacking into internal

### Are there any user-facing changes?

No

* GitHub Issue: apache#41116

Lead-authored-by: mwish <[email protected]>
Co-authored-by: mwish <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: mwish <[email protected]>
rok pushed a commit to tmct/arrow that referenced this issue May 8, 2024
@mapleFU @pitrou @rok
apacheGH-41116: [C++] IO: enhance boundary checking in CompressedInpu…
76500c6 
…tStream (apache#41117)

### Rationale for this change

Enhance the boundary checking code style in `io::CompressedInputStream`.

### What changes are included in this PR?

* Add `compressed_buffer_available` and `decompressed_buffer_available` in the class, and uses them for checking the boundary
* Change `Status(bool*)` to `Result<bool>`

### Are these changes tested?

Already has testing. I don't know how to hacking into internal

### Are there any user-facing changes?

No

* GitHub Issue: apache#41116

Lead-authored-by: mwish <[email protected]>
Co-authored-by: mwish <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: mwish <[email protected]>
rok pushed a commit to tmct/arrow that referenced this issue May 8, 2024
@mapleFU @pitrou @rok
apacheGH-41116: [C++] IO: enhance boundary checking in CompressedInpu…
c60c5d8 
…tStream (apache#41117)

### Rationale for this change

Enhance the boundary checking code style in `io::CompressedInputStream`.

### What changes are included in this PR?

* Add `compressed_buffer_available` and `decompressed_buffer_available` in the class, and uses them for checking the boundary
* Change `Status(bool*)` to `Result<bool>`

### Are these changes tested?

Already has testing. I don't know how to hacking into internal

### Are there any user-facing changes?

No

* GitHub Issue: apache#41116

Lead-authored-by: mwish <[email protected]>
Co-authored-by: mwish <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: mwish <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
@mapleFU @pitrou @vibhatha
apacheGH-41116: [C++] IO: enhance boundary checking in CompressedInpu…
32e2aee 
…tStream (apache#41117)

### Rationale for this change

Enhance the boundary checking code style in `io::CompressedInputStream`.

### What changes are included in this PR?

* Add `compressed_buffer_available` and `decompressed_buffer_available` in the class, and uses them for checking the boundary
* Change `Status(bool*)` to `Result<bool>`

### Are these changes tested?

Already has testing. I don't know how to hacking into internal

### Are there any user-facing changes?

No

* GitHub Issue: apache#41116

Lead-authored-by: mwish <[email protected]>
Co-authored-by: mwish <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: mwish <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mapleFU mapleFU

Labels
Component: C++ Type: enhancement
Projects
None yet
Milestone
17.0.0
Development

No branches or pull requests
1 participant
@mapleFU