Skip to content

Commit

Permalink
Bug 1690167 - Change VsprintfLiteral/SprintfLiteral to rely on Printf…
Browse files Browse the repository at this point in the history
…Target. r=nika,Gankra,firefox-build-system-reviewers,mhentges

Instead of snprintf.

Because some standalone code uses those functions directly or indirectly,
and PrintfTarget lives in mozglue, they now need to depend on mozglue
instead of mfbt. Except logalloc/replay, which cherry-picks what it
uses.

Differential Revision: https://phabricator.services.mozilla.com/D103730
  • Loading branch information
glandium committed Feb 16, 2021
1 parent 34cf11e commit 622b111
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 40 deletions.
2 changes: 1 addition & 1 deletion media/gmp-clearkey/0.1/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
with Files("**"):
BUG_COMPONENT = ("Core", "Audio/Video: GMP")

SharedLibrary("clearkey")
GeckoSharedLibrary("clearkey", linkage=None)

FINAL_TARGET = "dist/bin/gmp-clearkey/0.1"

Expand Down
14 changes: 13 additions & 1 deletion memory/replace/logalloc/replay/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,19 @@ if CONFIG["MOZ_DMD"] or CONFIG["MOZ_PHC"]:
"/mozglue/misc/StackWalk.h",
]

if not CONFIG["MOZ_REPLACE_MALLOC_STATIC"]:
if CONFIG["MOZ_REPLACE_MALLOC_STATIC"]:
UNIFIED_SOURCES += [
"/mfbt/double-conversion/double-conversion/bignum-dtoa.cc",
"/mfbt/double-conversion/double-conversion/bignum.cc",
"/mfbt/double-conversion/double-conversion/cached-powers.cc",
"/mfbt/double-conversion/double-conversion/double-to-string.cc",
"/mfbt/double-conversion/double-conversion/fast-dtoa.cc",
"/mfbt/double-conversion/double-conversion/fixed-dtoa.cc",
"/mfbt/double-conversion/double-conversion/string-to-double.cc",
"/mfbt/double-conversion/double-conversion/strtod.cc",
"/mozglue/misc/Printf.cpp",
]
else:
SOURCES += [
"../FdPrintf.cpp",
]
Expand Down
5 changes: 1 addition & 4 deletions mfbt/tests/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ if not CONFIG["MOZ_ASAN"]:
]
)

# Since we link directly with MFBT object files, define IMPL_MFBT
DEFINES["IMPL_MFBT"] = True

DisableStlWrapping()

if CONFIG["CC_TYPE"] == "clang-cl":
Expand All @@ -103,7 +100,7 @@ if CONFIG["CC_TYPE"] == "clang-cl":
]

USE_LIBS += [
"mfbt",
"mozglue",
]

if CONFIG["CC_TYPE"] in ("clang", "gcc"):
Expand Down
2 changes: 2 additions & 0 deletions mozglue/misc/Printf.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ class PrintfTarget {
bool MFBT_API appendIntOct(uint64_t);
bool MFBT_API appendIntHex(uint64_t);

inline size_t emitted() { return mEmitted; }

protected:
MFBT_API PrintfTarget();
virtual ~PrintfTarget() = default;
Expand Down
39 changes: 36 additions & 3 deletions mozglue/misc/Sprintf.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,52 @@

#include <stdio.h>
#include <stdarg.h>
#include <algorithm>

#include "mozilla/Assertions.h"
#include "mozilla/Attributes.h"
#include "mozilla/Printf.h"

#ifdef __cplusplus

namespace mozilla {
namespace detail {

struct MOZ_STACK_CLASS SprintfAppend final : public mozilla::PrintfTarget {
template <size_t N>
explicit SprintfAppend(char (&aBuf)[N]) : mBuf(aBuf), mBufLen(N) {}

bool append(const char* aStr, size_t aLen) override {
if (aLen == 0) {
return true;
}
// Don't copy more than what's left to use.
size_t copy = std::min(mBufLen, aLen);
if (copy > 0) {
memcpy(mBuf, aStr, copy);
mBuf += copy;
mBufLen -= copy;
}
return true;
}

private:
char* mBuf;
size_t mBufLen;
};

} // namespace detail
} // namespace mozilla

template <size_t N>
MOZ_FORMAT_PRINTF(2, 0)
int VsprintfLiteral(char (&buffer)[N], const char* format, va_list args) {
MOZ_ASSERT(format != buffer);
int result = vsnprintf(buffer, N, format, args);
buffer[N - 1] = '\0';
return result;
mozilla::detail::SprintfAppend ss(buffer);
ss.vprint(format, args);
size_t len = ss.emitted();
buffer[std::min(len, N - 1)] = '\0';
return len;
}

template <size_t N>
Expand Down
23 changes: 0 additions & 23 deletions toolkit/mozapps/update/updater/TsanOptions.cpp

This file was deleted.

8 changes: 1 addition & 7 deletions toolkit/mozapps/update/updater/updater-common.build
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ if CONFIG["OS_ARCH"] == "WINNT":
USE_LIBS += [
"bspatch",
"mar",
"mozglue",
"updatecommon",
"xz-embedded",
]
Expand Down Expand Up @@ -78,13 +79,6 @@ if have_progressui == 0:

SOURCES += sorted(srcs)

if CONFIG["MOZ_TSAN"]:
# Since mozglue is not linked to the updater,
# we need to include our own TSan suppression list.
SOURCES += [
"TsanOptions.cpp",
]

DEFINES["NS_NO_XPCOM"] = True
DisableStlWrapping()
for var in ("MAR_CHANNEL_ID", "MOZ_APP_VERSION"):
Expand Down
2 changes: 1 addition & 1 deletion xpcom/tests/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ test_progs = [
]
SimplePrograms(test_progs)

USE_LIBS += ["mfbt"]
USE_LIBS += ["mozglue"]

XPCSHELL_TESTS_MANIFESTS += ["unit/xpcshell.ini"]

Expand Down

0 comments on commit 622b111

Please sign in to comment.