refactor: Remove unused raw-pointer read helper from univalue #28168

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2307-remove-code- changing 13 files +27 −26

maflcko commented at 10:21 AM on July 27, 2023: member

The helpers are unused outside of tests and redundant with the existing bool read(std::string_view raw);.

Fix both issues by removing them.

Also, simplify the tests code by removing a std::string constructor where possible.
DrahtBot commented at 10:21 AM on July 27, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK stickies-v, TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Refactoring on Jul 27, 2023
fanquake requested review from stickies-v on Jul 27, 2023
maflcko force-pushed on Jul 27, 2023
in src/univalue/include/univalue.h:98 in fa783ac5c6 outdated
```
  93 | @@ -94,9 +94,7 @@ class UniValue {
  94 |      std::string write(unsigned int prettyIndent = 0,
  95 |                        unsigned int indentLevel = 0) const;
  96 |  
  97 | -    bool read(const char *raw, size_t len);
  98 | -    bool read(const char *raw) { return read(raw, strlen(raw)); }
```
stickies-v commented at 10:47 AM on July 27, 2023:

nit: can't get iwyu to work on this file, but i think #include <cstring> is now no longer required

maflcko commented at 1:19 PM on July 27, 2023:

can't get iwyu to work on this file

You can just use the output produced by CI (tidy)

but i think #include <cstring> is now no longer required

Thanks, done.
maflcko force-pushed on Jul 27, 2023
DrahtBot added the label CI failed on Jul 27, 2023
stickies-v approved
stickies-v commented at 11:14 AM on July 27, 2023: contributor

utACK 9999cfc1f3b728132e011f0aec24211212924de2
maflcko force-pushed on Jul 27, 2023

Type	Reviewers
ACK	stickies-v, TheCharlatan

in build_msvc/test_bitcoin/test_bitcoin.vcxproj:71 in fa7668e7a7 outdated

  67 | @@ -68,7 +68,7 @@
  68 |        <RawTestFile Include="..\..\src\test\data\*.raw" />
  69 |      </ItemGroup>
  70 |      <HeaderFromHexdump RawFilePath="%(RawTestFile.FullPath)" HeaderFilePath="%(RawTestFile.FullPath).h" SourceHeader="static unsigned const char %(RawTestFile.Filename)_raw[] = {" SourceFooter="};" />
  71 | -    <HeaderFromHexdump RawFilePath="%(JsonTestFile.FullPath)" HeaderFilePath="%(JsonTestFile.FullPath).h" SourceHeader="namespace json_tests{ static unsigned const char %(JsonTestFile.Filename)[] = {" SourceFooter="};}" />
  72 | +    <HeaderFromHexdump RawFilePath="%(JsonTestFile.FullPath)" HeaderFilePath="%(JsonTestFile.FullPath).h" SourceHeader="#include <string> /*\n*/ namespace json_tests{ static const std::string %(JsonTestFile.Filename){" SourceFooter="};}" />

maflcko commented at 11:57 AM on July 27, 2023:

cc @sipsorcery @hebasto Any idea how to make a newline after the #include statement here on Windows?

stickies-v commented at 12:18 PM on July 27, 2023:

What about this? Adds a CR/LF so should work on windows? (but can't test)

<HeaderFromHexdump RawFilePath="%(JsonTestFile.FullPath)" HeaderFilePath="%(JsonTestFile.FullPath).h" SourceHeader="#include &lt;string&gt;&#x0D;&#x0A;namespace json_tests{ static const std::string %(JsonTestFile.Filename){" SourceFooter="};}" />

maflcko commented at 1:18 PM on July 27, 2023:

Thanks, no idea if and why it worked, but the Windows CI passed the step.

maflcko marked this as a draft on Jul 27, 2023
Remove unused raw-pointer read helper from univalue fa940f41ea
maflcko force-pushed on Jul 27, 2023
maflcko marked this as ready for review on Jul 27, 2023
stickies-v approved
stickies-v commented at 1:19 PM on July 27, 2023: contributor

utACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8
DrahtBot removed the label CI failed on Jul 27, 2023
TheCharlatan approved
TheCharlatan commented at 10:11 AM on July 28, 2023: contributor

tACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8
fanquake merged this on Jul 28, 2023
fanquake closed this on Jul 28, 2023
maflcko deleted the branch on Jul 28, 2023
sidhujag referenced this in commit 0b17a2d187 on Aug 9, 2023
Fabcien referenced this in commit d3460c587e on Jun 6, 2024
bitcoin locked this on Jul 27, 2024

Contributors

Labels

refactor: Remove unused raw-pointer read helper from univalue #28168

Reviews