Page MenuHomePhabricator

[libcxx] Fix the format strings for formatting paths in directory_iterator.cpp
AbandonedPublic

Authored by mstorsjo on Mar 5 2021, 1:34 PM.

Details

Reviewers
Quuxplusone
Group Reviewers
Restricted Project

Diff Detail

Event Timeline

mstorsjo requested review of this revision.Mar 5 2021, 1:34 PM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2021, 1:34 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Quuxplusone added inline comments.Mar 5 2021, 3:18 PM
libcxx/src/filesystem/filesystem_common.h
115

How about moving the escaped-double-quotes into this format string?

#define PS_FMT "\"%ls\""
#define PS_FMT "\"%s\""

That'll make the call-sites look less wacky.

(I admit it doesn't really make any sense that we double-quote these paths in the first place. It's not as if we're going to actually escape the backslashes/quotes in the paths; so the quotation marks are kind of a lie. But I don't feel like unilaterally suggesting their complete removal.)

curdeius added inline comments.
libcxx/src/filesystem/filesystem_common.h
115

Personally I'd go the opposite way and rather see these macros closer to PRIxYYx macros (cf. https://en.cppreference.com/w/c/types/integer) and so not even include the % sign. This way, they could be used in the future with additional format options, like width, between % and the macro.

#define PS_FMT "ls"
#define PS_FMT "s"

And put % in the format string. WDYT?

mstorsjo added inline comments.Mar 5 2021, 11:33 PM
libcxx/src/filesystem/filesystem_common.h
115

Something along those lines were my original intent, but I don't object to going the other way and including more parts in the macro.

I'll leave resolving the rest of the bits in D98097 to you, thanks!

mstorsjo abandoned this revision.Mar 16 2021, 12:04 PM

Superseded by D98097.