GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — resetDecoder should be called once per decoding by miniway · Pull Request #387 · msgpack/msgpack-java

Conversation

@miniway

@miniway miniway commented Sep 23, 2016

Copy link
Copy Markdown
Contributor

When multiple MessageBuffer involved at unpackString first bytes can be removed because resetDecoder is called multiple times.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
xerial
xerial reviewed Sep 23, 2016
View reviewed changes
Comment thread msgpack-core/src/main/java/org/msgpack/core/buffer/SequenceMessageBufferInput.java Outdated
this.sequence = checkNotNull(sequence, "input sequence is null");
try {
nextInput();
} catch (IOException ignore) {

@xerial xerial Sep 23, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style check error: https://travis-ci.org/msgpack/msgpack-java/jobs/162190720#L507

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@miniway
resetDecoder should be called once per decoding
9467f61
@miniway
miniway force-pushed the fix_truncate branch from f634f34 to 9467f61 Compare September 23, 2016 16:30
xerial
xerial reviewed Sep 23, 2016
View reviewed changes
Comment thread msgpack-core/src/test/scala/org/msgpack/core/MessageUnpackerTest.scala
}

"read numeric data at buffer boundary" taggedAs("boundary2") in {
"read data at buffer boundary" taggedAs("boundary2") in {

@xerial xerial Sep 23, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case is intended for checking a bug on unpackInteger that found in the past. We should have another test case for string.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@xerial
xerial merged commit 9467f61 into msgpack:develop Sep 23, 2016
xerial added a commit that referenced this pull request Sep 23, 2016
@xerial
Merge pull request #388 from msgpack/miniway-fix_truncate
f16e762 
Follow-up of #387
@xerial

xerial commented Sep 23, 2016

Copy link
Copy Markdown
Member

Merged in #388
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
frsyuki
frsyuki reviewed Sep 23, 2016
View reviewed changes
Comment thread msgpack-core/src/main/java/org/msgpack/core/buffer/SequenceMessageBufferInput.java
private Enumeration<? extends MessageBufferInput> sequence;
private MessageBufferInput input;

public SequenceMessageBufferInput(Enumeration<? extends MessageBufferInput> sequence)

@frsyuki frsyuki Sep 23, 2016 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use Iterator, Stream, Iterable, or Collection instead of Enumeration? or how about moving this to test package only for now? Because Enumeration is considered as a deprecated interface used only by old classes such as Vector or Hashtable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@xerial xerial Sep 23, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Iterator<MessageBufferInput> would work. And this should be in test package.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@frsyuki frsyuki mentioned this pull request Sep 23, 2016
Mimic SequenceInputStream interface more in SequenceMessageBufferInput #389
Closed
@xerial xerial mentioned this pull request Sep 23, 2016
Upgrade to msgpack-java 0.8.10 treasure-data/td-client-java#74
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@frsyuki frsyuki frsyuki left review comments
@xerial xerial xerial left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@miniway @xerial @frsyuki
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.