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

curl_easy_setopt man page: Clarification about passed in string storage #7632

Closed
yaobinwen opened this issue Aug 25, 2021 · 1 comment
Closed

Comments

@yaobinwen
Copy link

@yaobinwen yaobinwen commented Aug 25, 2021

Hi,

I see many other issues use a template that has "I did this" and "I expect the following" to report bugs. This issue is not about a bug but I'm hoping for some clarification about the document of curl_easy_setopt. So I'm not following that convention. The link to the code is here.

My Confusion

The doc says:

Strings passed to libcurl as 'char *' arguments, are copied by the library; thus the string storage associated to the pointer argument may be overwritten after curl_easy_setopt returns.

I was confused by the words "... may be overwritten ..." when I initially read it because the sentence doesn't explicitly mention the actor of "overwrite". As a result, being a non-English speaker myself, my first impression was "the string storage could be overwritten (by curl_easy_setopt) after curl_easy_setopt returns", which confused me a little bit because the previous sentence just said "copied by the library".

After reading the entire page, I think the words mean:

  • 1). libcurl creates a copy of the strings.
  • 2). When curl_easy_setopt returns, the caller doesn't have to keep the string storage available and can either discard the string storage or overwrite it with other content.

Am I understanding the document correctly?

My Suggestion

If my understanding above is correct, can we refine the document as follows?

Firstly, avoid using "... may be overwritten ..." but clearly state what the programmer can do with the string storage. Something like below:

Strings passed to libcurl as 'char *' arguments, are copied by the library; thus the user can either discard the string storage or reuse it for other content after curl_easy_setopt returns.

Or, if passive voice is preferred, the doc could be refined as follows:

Strings passed to libcurl as 'char *' arguments, are copied by the library; thus the string storage associated to the pointer argument may be either discarded or reused for other content after curl_easy_setopt returns.

Secondly, put the note "Before version 7.17.0, strings were not copied." right after the sentence above in order to create a closer link. So my final suggestion is:

(In active voice)

Strings passed to libcurl as 'char *' arguments, are copied by the library; thus the user can either discard the string storage or reuse it for other things after curl_easy_setopt returns. In contrast, before version 7.17.0, strings were not copied so the user was forced keep them available until libcurl no longer needed them. The only exception to this rule is ... (the rest of the paragraph).

(In passive voice)

Strings passed to libcurl as 'char *' arguments, are copied by the library; thus the string storage associated to the pointer argument may be either discarded or reused for other content after curl_easy_setopt returns. In contrast, before version 7.17.0, strings were not copied so the user was forced keep them available until libcurl no longer needed them. The only exception to this rule is ... (the rest of the paragraph).

@bagder
Copy link
Member

@bagder bagder commented Aug 26, 2021

put the note "Before version 7.17.0, strings were not copied." right after the sentence

I don't think this piece is necessary. Primarily because 7.17.0 is such an old version that this basically doesn't affect any existing user.

bagder added a commit that referenced this issue Aug 26, 2021
Reported-by: Yaobin Wen
Fixes #7632
@bagder bagder closed this in 1a042be Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants