New issue
Advanced search Search tips

Issue 882796 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 3
Type: Task



Sign in to add a comment

base/file_path.h defines both PRIsFP and PRFilePath

Project Member Reported by sdefresne@chromium.org, Sep 11

Issue description

base/file_path.h defines two macros with exactly the same value and for the same use case of providing the %-format for use in printf-like format function. They are called PRIsFP (11 references on codesearch) and PRFilePath (24 references on codesearch).


// To print path names portably use PRIsFP (based on PRIuS and friends from
// C99 and format_macros.h) like this:
// base::StringPrintf("Path is %" PRIsFP ".\n", path.value().c_str());
#if defined(OS_WIN)
#define PRIsFP "ls"
#elif defined(OS_POSIX) || defined(OS_FUCHSIA)
#define PRIsFP "s"
#endif  // OS_WIN

// Macros for string literal initialization of FilePath::CharType[], and for
// using a FilePath::CharType[] in a printf-style format string.
#if defined(OS_WIN)
#define FILE_PATH_LITERAL(x) L ## x
#define PRFilePath "ls"
#elif defined(OS_POSIX) || defined(OS_FUCHSIA)
#define FILE_PATH_LITERAL(x) x
#define PRFilePath "s"
#endif  // OS_WIN

We should probably remove one of the two macros.
 
I would like to fix this issue if no one working on this. :)
I don't think anyone has claimed it. Feel free to work on this. Thank you.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600

commit 4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600
Author: Jaeyong Bae <jdragon.bae@gmail.com>
Date: Sat Sep 29 04:50:19 2018

Remove PRIsFP macro in base/file_path.h

This CL means removing duplicate macro in base/file_path.h.
PRIsFP is replaced with PRFilePath.

Bug:  882796 
Change-Id: I69f1dc3c13921d0f5d8f1f2de577f27f8db072f4
Reviewed-on: https://chromium-review.googlesource.com/1220406
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595301}
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/base/files/file_path.h
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/chrome/browser/sync_file_system/drive_backend/local_to_remote_syncer.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/chrome/common/extensions/api/storage/storage_schema_manifest_handler.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/chrome/services/file_util/public/cpp/sandboxed_rar_analyzer.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/chrome/test/chromedriver/element_commands.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/components/policy/core/common/preg_parser.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/components/reading_list/core/offline_url_utils_unittest.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/components/safe_browsing/db/v4_protocol_manager_util.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/components/safe_browsing/db/v4_store.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/content/browser/devtools/protocol/memory_handler.cc
[modify] https://crrev.com/4f998e0cfe8fe1ccbb2a1da875c8e817ffe1d600/storage/browser/fileapi/dump_file_system.cc

Owner: jdragon....@gmail.com
Status: Fixed (was: Available)
Thank you for the fix.

Sign in to add a comment