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.

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

libxo output breaks if buffer exceeds 8kb #37

Closed
allanjude opened this issue May 4, 2015 · 4 comments
Closed

libxo output breaks if buffer exceeds 8kb #37

allanjude opened this issue May 4, 2015 · 4 comments

Comments

@allanjude
Copy link
Contributor

While converting the FreeBSD procstat utility to libxo, I noticed that if the output exceeds 8kb, and stdout is a file or a pipe, the content is truncated.

to reproduce, run some command that produces more than 8192 bytes of data via libxo, and redirect stdout to a file, or pipe it to less or similar. The output will be truncated.

@allanjude
Copy link
Contributor Author

I could be wrong, but it looks like the problem is caused by the call to xo_buf_has_room() not considering some part of the pretty printing:

ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8154, len: 12
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8166, len: 1
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8167, len: 5
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8172, len: 2
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8174, len: 1
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8175, len: 5
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8212, len: 12
ALLAN: Expanded buffer
ALLAN: xo_buf_has_room: xb_size: 16384, curp: 8224, len: 1
ALLAN: xo_buf_has_room: xb_size: 16384, curp: 8225, len: 4

It doesn't appear to expand the buffer until the cur pointer has passed the end of the buffer.

@philshafer
Copy link

Could you please retry with the current code? I fixed an embarrassingly bad issue in xo_vsnprintf:

  • if (rc > xbp->xb_size) {
  • if (rc >= left) {

in commit 1f66642 that’s part of libxo-0.3.3. IIRC the fix is in 0.3.2 also.

But I’m fairly confident this was the issue you are seeing.

Thanks,
Phil

On May 4, 2015, at 12:40 AM, Allan Jude notifications@github.com wrote:

I could be wrong, but it looks like the problem is caused by the call to xo_buf_has_room() not considering some part of the pretty printing:

ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8154, len: 12
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8166, len: 1
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8167, len: 5
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8172, len: 2
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8174, len: 1
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8175, len: 5
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8212, len: 12
ALLAN: Expanded buffer
ALLAN: xo_buf_has_room: xb_size: 16384, curp: 8224, len: 1
ALLAN: xo_buf_has_room: xb_size: 16384, curp: 8225, len: 4

It doesn't appear to expand the buffer until the cur pointer has passed the end of the buffer.


Reply to this email directly or view it on GitHub #37 (comment).

@allanjude
Copy link
Contributor Author

I have 0.3.2 with that fix.

The issue seems to be that the exact same bug exists in:
xo_printf_v()

ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8166, len: 1
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8167, len: 5
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8172, len: 2
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8174, len: 1
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8175, len: 5
ALLAN: xo_printf_v: wrote 12 of 12 bytes
ALLAN: xo_printf_v: wrote 20 of 0 bytes
ALLAN: xo_buf_has_room: xb_size: 8192, curp: 8212, len: 12
ALLAN: Expanding Buffer
ALLAN: xo_buf_has_room: xb_size: 16384, curp: 8224, len: 1
ALLAN: xo_buf_has_room: xb_size: 16384, curp: 8225, len: 4

allanjude added a commit to allanjude/libxo that referenced this issue May 4, 2015
When checking the return value of vsnprintf, these functions compared against the size of the buffer, rather than the number of bytes remaining in the buffer, and so fail to grow the buffer when it was insufficient to fit the new content.

Related to: 1f66642
Fixes: Juniper#37
@philshafer
Copy link

Fixed in 0.3.4

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

No branches or pull requests

2 participants