doc: Add developer note on c_str() #17281

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2019_10_c_str_note changing 1 files +22 −0

laanwj commented at 1:08 PM on October 28, 2019: member

Add a note when to use and when not to use c_str().
laanwj added the label Docs on Oct 28, 2019
in doc/developer-notes.md:661 in 217e4a5304 outdated
```
 656 | +  - Do not use it to convert to `QString`. Use `QString::fromStdString()`.
 657 | +
 658 | +    - *Rationale*: Qt has build-in functionality for converting their string
 659 | +      type from/to C++. No need to roll your own.
 660 | +
 661 | +  - You might want to do extra checking before passing strings that can contain embedded NULL characters, because
```
ryanofsky commented at 3:59 PM on October 28, 2019:

Maybe change to:

In cases where do you call .c_str(), you might want to additionally check that the string does not contain embedded '\0' characters[...]

I was initially confused about what this bullet point what referring to after reading the previous three "Do not use it..." points.
laanwj commented at 7:24 PM on October 28, 2019:

Will do
ryanofsky approved
ryanofsky commented at 4:02 PM on October 28, 2019: member

ACK 217e4a5304e3a042eb4983b48823bcfb7cb010e3. All good notes.
emilengler commented at 5:49 PM on October 28, 2019: contributor

ACK
in doc/developer-notes.md:650 in 217e4a5304 outdated
```
 645 | +
 646 | +  - Do not use it when passing a sized array (so along with `.size()`). Use `.data()` instead to get a pointer
 647 | +    to the raw data.
 648 | +
 649 | +    - *Rationale*: This can be dangerous, as the part of the data after the first NULL character is not guaranteed
 650 | +    to be initialized at all. Also, `.data()` communicates the intent better.
```
MarcoFalke commented at 7:07 PM on October 28, 2019:

Is this true after C++11? According to https://en.cppreference.com/w/cpp/string/basic_string/c_str

c_str() and data() perform the same function. | (since C++11)

laanwj commented at 7:24 PM on October 28, 2019:

Ok, I didn't know that, I'll remove that from the rationale. But I'd like to keep the suggestion because I still think it's bad form to use c_str in these cases.
laanwj referenced this in commit 5728f88d64 on Oct 30, 2019

doc: Add developer note on c_str()

Add a note when to use and when not to use `c_str()`.

1cf9b35c0d

laanwj force-pushed on Oct 30, 2019
laanwj commented at 9:53 AM on October 30, 2019: member

Addressed comments and squashed.
elichai commented at 1:00 PM on October 30, 2019: contributor

ACK 1cf9b35c0dac5f685b7ae62ded16284803816570 I agree that unless you pass a string to C there's almost always a better way to do it in C++. (and if you need array I agree with you that .data() is more self explanatory)
MarcoFalke commented at 1:15 PM on October 30, 2019: member

Looking nice ACK 1cf9b35c0dac5f685b7ae62ded16284803816570
laanwj referenced this in commit a6abc94e93 on Oct 30, 2019
laanwj merged this on Oct 30, 2019
laanwj closed this on Oct 30, 2019
sidhujag referenced this in commit d7a4574b8a on Oct 30, 2019
sidhujag referenced this in commit 6b20919659 on Oct 31, 2019

in doc/developer-notes.md:643 in 1cf9b35c0d

 639 | @@ -640,6 +640,28 @@ Strings and formatting
 640 |  
 641 |    - *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion.
 642 |  
 643 | +- Use `.c_str()` sparingly. Its only valid use is to pass C++ strings to C functions that take NULL-terminated

ryanofsky commented at 7:52 PM on November 5, 2019:

Could say null-terminated instead of NULL-terminated. NULL constant is technically unrelated

laanwj commented at 8:01 PM on November 5, 2019:

It's the NULL or NUL character, too. C's NULL constant is indeed unrelated.

jasonbcox referenced this in commit 81d108f6f2 on Sep 9, 2020
sidhujag referenced this in commit 401599fe37 on Nov 10, 2020
sidhujag referenced this in commit 4b24b550c2 on Nov 10, 2020
PastaPastaPasta referenced this in commit 413d595693 on Jun 27, 2021
PastaPastaPasta referenced this in commit 15a0d9a0b2 on Jun 28, 2021
PastaPastaPasta referenced this in commit 5dd9247dfd on Jun 29, 2021
PastaPastaPasta referenced this in commit b1fc3d2252 on Jul 1, 2021
PastaPastaPasta referenced this in commit 11a42a1041 on Jul 1, 2021
PastaPastaPasta referenced this in commit c5c6a453de on Sep 11, 2021
PastaPastaPasta referenced this in commit b618babd0f on Sep 11, 2021
PastaPastaPasta referenced this in commit b1aba45a4b on Sep 12, 2021
PastaPastaPasta referenced this in commit 688a23ee3f on Sep 12, 2021
PastaPastaPasta referenced this in commit fe45b93cc0 on Sep 12, 2021
PastaPastaPasta referenced this in commit 93f983d4a5 on Sep 14, 2021
PastaPastaPasta referenced this in commit ba1d5581cb on Sep 14, 2021
PastaPastaPasta referenced this in commit 46c3eaab48 on Sep 15, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#17280 refactor: Change occurences of c_str() used with size() to data()