Skip to content

cli tool: reimplement stdin buffering in -F. - #1985

Closed
monnerat wants to merge 2 commits into
curl:masterfrom
monnerat:buffer_stdin
Closed

cli tool: reimplement stdin buffering in -F.#1985
monnerat wants to merge 2 commits into
curl:masterfrom
monnerat:buffer_stdin

Conversation

@monnerat

Copy link
Copy Markdown
Contributor

Dummy PR to check new code on APPVeyor/Travis CI

Comment thread src/tool_formparse.c
/* Check if file is "-". If so, use a callback to read OUR stdin (to

/* Mime part callbacks for stdin. */
static size_t stdin_read(char *buffer, size_t size, size_t nitems, void *arg)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's already a file2memory function you should use instead!

Comment thread src/tool_formparse.c
}
/* Shrink buffer to spare memory resources. */
if(sip->size < sip->curpos)
sip->data = realloc(sip->data, (size_t) sip->size);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Even shrinking can fail so this return code must be checked and bail out if you fail.

@monnerat

Copy link
Copy Markdown
Contributor Author

Pushed: 665b3e4

@monnerat monnerat closed this Oct 12, 2017
@monnerat
monnerat deleted the buffer_stdin branch October 12, 2017 13:38
@bagder

bagder commented Oct 12, 2017

Copy link
Copy Markdown
Member

Yes, but none of my remarks were addressed?

And please use 'closes #[num]' in commit messages that close PRs (or "closes [URL]")

@bagder

bagder commented Oct 12, 2017

Copy link
Copy Markdown
Member

oh, did I remark after it was merged perhaps? I think you need to give us slightly more time to review...

@monnerat

monnerat commented Oct 12, 2017

Copy link
Copy Markdown
Contributor Author

oh, did I remark after it was merged perhaps? I think you need to give us slightly more time to review...

Yes, I think so: I've just received the mails.

Thanks for your review and I'll take your remarks in consideration. I was not expecting it but just wanted to have checks on cross-platforms :-)

monnerat added a commit that referenced this pull request Oct 12, 2017
@monnerat

Copy link
Copy Markdown
Contributor Author

There's already a file2memory function you should use instead!

Fixed: 5650905

@lock lock Bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants