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.

Sign up for GitHub

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

Jump to bottom

fix #4569: clang warning about deprecated sprintf usage. #4570

Closed
wants to merge 34 commits into from
Closed

fix #4569: clang warning about deprecated sprintf usage. #4570

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7b481f0
fix #4569: clang warning about deprecated sprintf usage.
ckeshava Jun 12, 2023
2bf715b
Merge branch 'develop' into sprintfFix
ckeshava Jul 6, 2023
d6b06bc
Merge branch 'develop' into sprintfFix
ckeshava Jul 10, 2023
aa5ab30
fix #4569: clang warning about deprecated sprintf usage.
ckeshava Jun 12, 2023
a46f1e7
Trivial: add comments for NFToken-related invariants (#4558)
scottschurr Jun 9, 2023
80c608b
fix node size estimation (#4536)
dangell7 Jun 9, 2023
6c424ca
fix: remove redundant moves (#4565)
ckeshava Jun 10, 2023
470779a
Set version to 1.11.0-rc3
intelliot Jun 9, 2023
eaf58f2
Set version to 1.11.0
intelliot Jun 20, 2023
bd85cfa
Enable the Beta RPC API (v2) for all unit tests: (#4573)
ximinez Jun 21, 2023
11d2723
`fixReducedOffersV1`: prevent offers from blocking order books: (#4512)
scottschurr Jun 23, 2023
5a7d35c
Add RPC/WS ports to server_info (#4427)
drlongle Jun 23, 2023
238a248
ci: use Artifactory remote in nix workflow (#4556)
thejohnfreeman Jun 23, 2023
8520356
refactor: remove TypedField's move constructor (#4567)
HowardHinnant Jun 26, 2023
0f31c65
XLS-39 Clawback: (#4553)
shawnxie999 Jun 26, 2023
3a7b98e
Set version to 1.12.0-b1
intelliot Jun 26, 2023
29e15f4
refactor: replace hand-rolled lexicalCast (#4473)
dangell7 Jun 27, 2023
67289be
Fix package definition for Conan (#4485)
thejohnfreeman Jun 27, 2023
284bc4a
fix: remove deprecated fields in `ledger` method (#4244)
ckeshava Jun 28, 2023
7a50fee
refactor: rename ServerHandlerImp to ServerHandler (#4516)
scottschurr Jun 28, 2023
18193dc
Fix build references to deleted ServerHandlerImp: (#4592)
ximinez Jun 28, 2023
c9f7312
APIv2: add error messages for account_tx (#4571)
PeterChen13579 Jun 29, 2023
a197d5b
fix: Update Handler::Condition enum values #3417 (#4239)
ckeshava Jun 29, 2023
671f5df
ci: cancel overridden workflows (#4597)
thejohnfreeman Jun 29, 2023
25c03c6
fix: deb package build (#4591)
legleux Jun 30, 2023
be217d9
APIv2(account_info): handle invalid "signer_lists" value (#4585)
PeterChen13579 Jun 30, 2023
8dcbbf1
chore: update checkout action version to v3: (#4598)
ximinez Jul 1, 2023
0ab8147
add clang-format pre-commit hook (#4599)
mvadari Jul 1, 2023
d2f1759
build: add binary hardening compile and link flags (#4603)
thejohnfreeman Jul 3, 2023
a6664eb
fix: add allowClawback flag for `account_info` (#4590)
shawnxie999 Jul 5, 2023
95d67eb
refactor: change the return type of mulDiv to std::optional (#4243)
ckeshava Jul 6, 2023
340b0b5
APIv2(ledger_entry): return "invalidParams" when fields missing (#4552)
arihantkothari Jul 6, 2023
3d4651d
docs(CONTRIBUTING): push beta releases to `release` (#4589)
intelliot Jul 7, 2023
1f66cac
Merge branch 'sprintfFix' of https:/ckeshava/rippled into…
ckeshava Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/ripple/basics/impl/contract.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <ripple/basics/Log.h>
#include <ripple/basics/contract.h>
#include <cstdlib>
#include <exception>
#include <iostream>

namespace ripple {
Expand Down
6 changes: 2 additions & 4 deletions src/ripple/json/impl/json_reader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

#include <algorithm>
#include <cctype>
#include <cstdint>
#include <istream>
#include <string>

Expand Down Expand Up @@ -924,9 +923,8 @@ Reader::getLocationLineAndColumn(Location location) const
{
int line, column;
getLocationLineAndColumn(location, line, column);
char buffer[18 + 16 + 16 + 1];
sprintf(buffer, "Line %d, Column %d", line, column);
return buffer;
return "Line " + std::to_string(line) + ", Column " +
std::to_string(column);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does multiple allocations. Consider using snprintf instead. Something like:

    int line, column;
    getLocationLineAndColumn(location, line, column);
    std::size_t n = 18 + 16 + 16 + 1;
    char buffer[n];
    snprintf(buffer, n, "Line %d, Column %d", line, column);
    return buffer;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay. I tried to do this in my first commit. But @a-noni-mousse had reservations with this approach (#4570 (comment))

Should I still go ahead with the snprintf approach?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't a performance critical function, the extra allocations aren't that big a deal. On the other hand, I don't find the snprintf dangerous here - it's clearly not going to overwrite the buffer and it's very easy to see that. So the issues I'd weigh are:

  • 4 memory allocations vs 1 memory allocation
  • a high level implementation vs a low level implementation
  • this is not a performance critical function, and is unlikely to ever be performance critical

It's your call, I'll approve this with either interface.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm okay, thanks for the comparison.

I feel the snprintf adds complexity. I don't understand why we need to allocate 18 + 16 + 16 + 1 as the buffer size (I mimicked the existing code's behavior)

I'm inclined the favor the std::string version for it's simplicity.

}

std::string
Expand Down