Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

agent: Fix the amount of bytes transmitted to the shim #124

Merged
merged 1 commit into from
Jan 30, 2018

Conversation

sboeuf
Copy link

@sboeuf sboeuf commented Jan 29, 2018

Every time our GRPC functions ReadStdout() or ReadStderr() were
called, the entire allocated buffer was returned. This is wrong
since we don't know the amount of bytes we will get on both stdout
and stderr. Instead, this patch returns the slice corresponding to
the real amount of bytes read from the process.

Fixes #123
Fixes kata-containers/runtime#20

Signed-off-by: Sebastien Boeuf sebastien.boeuf@intel.com

Every time our GRPC functions ReadStdout() or ReadStderr() were
called, the entire allocated buffer was returned. This is wrong
since we don't know the amount of bytes we will get on both stdout
and stderr. Instead, this patch returns the slice corresponding to
the real amount of bytes read from the process.

Fixes kata-containers#123

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
@sboeuf
Copy link
Author

sboeuf commented Jan 29, 2018

Relates to kata-containers/runtime#20

@codecov
Copy link

codecov bot commented Jan 29, 2018

Codecov Report

Merging #124 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   75.96%   75.96%           
=======================================
  Files           1        1           
  Lines         129      129           
=======================================
  Hits           98       98           
  Misses         16       16           
  Partials       15       15

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b89fa7c...8fba14f. Read the comment docs.

@sboeuf
Copy link
Author

sboeuf commented Jan 29, 2018

@egernst
Copy link
Member

egernst commented Jan 29, 2018

nice catch.

@WeiZhang555
Copy link
Member

WeiZhang555 commented Jan 30, 2018

LGTM

Approved with PullApprove

@gnawux
Copy link
Member

gnawux commented Jan 30, 2018

+1
nice

Approved with PullApprove

@bergwolf
Copy link
Member

bergwolf commented Jan 30, 2018

LGTM!

Approved with PullApprove

@bergwolf bergwolf merged commit b834af2 into kata-containers:master Jan 30, 2018
@grahamwhaley
Copy link
Contributor

Thanks for finding that @sboeuf :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants