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

Avoid zero-length memcpy in formatting #85391

Merged
merged 2 commits into from
May 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions library/alloc/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2323,9 +2323,10 @@ impl<T: fmt::Display + ?Sized> ToString for T {
// to try to remove it.
#[inline]
default fn to_string(&self) -> String {
use fmt::Write;
let mut buf = String::new();
buf.write_fmt(format_args!("{}", self))
let mut formatter = core::fmt::Formatter::new(&mut buf);
// Bypass format_args!() to avoid write_str with zero-length strs
fmt::Display::fmt(self, &mut formatter)
.expect("a Display implementation returned an error unexpectedly");
buf
}
Expand Down
40 changes: 29 additions & 11 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,28 @@ pub struct Formatter<'a> {
buf: &'a mut (dyn Write + 'a),
}

impl<'a> Formatter<'a> {
/// Creates a new formatter with default settings.
///
/// This can be used as a micro-optimization in cases where a full `Arguments`
/// structure (as created by `format_args!`) is not necessary; `Arguments`
/// is a little more expensive to use in simple formatting scenarios.
///
/// Currently not intended for use outside of the standard library.
#[unstable(feature = "fmt_internals", reason = "internal to standard library", issue = "none")]
#[doc(hidden)]
pub fn new(buf: &'a mut (dyn Write + 'a)) -> Formatter<'a> {
Formatter {
flags: 0,
fill: ' ',
align: rt::v1::Alignment::Unknown,
width: None,
precision: None,
buf,
}
}
}

// NB. Argument is essentially an optimized partially applied formatting function,
// equivalent to `exists T.(&T, fn(&T, &mut Formatter<'_>) -> Result`.

Expand Down Expand Up @@ -1075,22 +1097,16 @@ pub trait UpperExp {
/// [`write!`]: crate::write!
#[stable(feature = "rust1", since = "1.0.0")]
pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result {
let mut formatter = Formatter {
flags: 0,
width: None,
precision: None,
buf: output,
align: rt::v1::Alignment::Unknown,
fill: ' ',
};

let mut formatter = Formatter::new(output);
let mut idx = 0;

match args.fmt {
None => {
// We can use default formatting parameters for all arguments.
for (arg, piece) in iter::zip(args.args, args.pieces) {
formatter.buf.write_str(*piece)?;
if !piece.is_empty() {
formatter.buf.write_str(*piece)?;
}
(arg.formatter)(arg.value, &mut formatter)?;
idx += 1;
}
Expand All @@ -1099,7 +1115,9 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result {
// Every spec has a corresponding argument that is preceded by
// a string piece.
for (arg, piece) in iter::zip(fmt, args.pieces) {
formatter.buf.write_str(*piece)?;
if !piece.is_empty() {
formatter.buf.write_str(*piece)?;
}
// SAFETY: arg and args.args come from the same Arguments,
// which guarantees the indexes are always within bounds.
unsafe { run(&mut formatter, arg, &args.args) }?;
Expand Down