Skip to content

Commit a1c8b78

Browse files
slachiewiczclaude
authored andcommitted
THRIFT-6213: Define the TConfiguration default constants in the library
Client: cpp An odr-used static const data member needs this out-of-line definition in every C++ standard; drop it only once the constants become inline constexpr. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ad915ad commit a1c8b78

6 files changed

Lines changed: 51 additions & 0 deletions

File tree

‎lib/cpp/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ endif()
3030
# Create the thrift C++ library
3131
set(thriftcpp_SOURCES
3232
src/thrift/TApplicationException.cpp
33+
src/thrift/TConfiguration.cpp
3334
src/thrift/TOutput.cpp
3435
src/thrift/TUuid.cpp
3536
src/thrift/async/TAsyncChannel.cpp

‎lib/cpp/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ AM_CPPFLAGS = $(BOOST_CPPFLAGS) $(OPENSSL_INCLUDES) -I$(srcdir)/src -D__STDC_FOR
5656
# Define the source files for the module
5757

5858
libthrift_la_SOURCES = src/thrift/TApplicationException.cpp \
59+
src/thrift/TConfiguration.cpp \
5960
src/thrift/TOutput.cpp \
6061
src/thrift/TUuid.cpp \
6162
src/thrift/VirtualProfiling.cpp \

‎lib/cpp/libthrift.vcxproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
<ClCompile Include="src\thrift\server\TThreadedServer.cpp" />
5757
<ClCompile Include="src\thrift\server\TThreadPoolServer.cpp" />
5858
<ClCompile Include="src\thrift\TApplicationException.cpp" />
59+
<ClCompile Include="src\thrift\TConfiguration.cpp" />
5960
<ClCompile Include="src\thrift\TOutput.cpp" />
6061
<ClCompile Include="src\thrift\TUuid.cpp" />
6162
<ClCompile Include="src\thrift\transport\SocketCommon.cpp" />

‎lib/cpp/libthrift.vcxproj.filters‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
<ClCompile Include="src\thrift\TUuid.cpp" />
88
<ClCompile Include="src\thrift\TOutput.cpp" />
99
<ClCompile Include="src\thrift\TApplicationException.cpp" />
10+
<ClCompile Include="src\thrift\TConfiguration.cpp" />
1011
<ClCompile Include="src\thrift\transport\TTransportException.cpp">
1112
<Filter>transport</Filter>
1213
</ClCompile>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include <thrift/TConfiguration.h>
21+
22+
namespace apache {
23+
namespace thrift {
24+
25+
// Binding one of these constants to a reference, as std::make_shared or
26+
// std::min do, needs this definition for as long as the header declares them
27+
// static const rather than inline constexpr, in every C++ standard.
28+
const int TConfiguration::DEFAULT_MAX_MESSAGE_SIZE;
29+
const int TConfiguration::DEFAULT_MAX_FRAME_SIZE;
30+
const int TConfiguration::DEFAULT_RECURSION_DEPTH;
31+
32+
}
33+
} // apache::thrift

‎lib/cpp/test/TTransportFactoryConfigTest.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,3 +349,17 @@ BOOST_AUTO_TEST_CASE(test_framed_transport_default_constructor_initialises_max_f
349349
TFramedTransport wrapped(std::make_shared<TMemoryBuffer>(), config);
350350
BOOST_CHECK_EQUAL(wrapped.getMaxFrameSize(), static_cast<uint32_t>(kMaxFrameSize));
351351
}
352+
353+
// Binding a default to a reference or taking its address needs the library to
354+
// define it. Reading the addresses through volatile pointers keeps an
355+
// optimising compiler from folding the references away (THRIFT-6213).
356+
BOOST_AUTO_TEST_CASE(test_configuration_defaults_are_defined) {
357+
const int* volatile maxMessageSize = &TConfiguration::DEFAULT_MAX_MESSAGE_SIZE;
358+
const int* volatile maxFrameSize = &TConfiguration::DEFAULT_MAX_FRAME_SIZE;
359+
const int* volatile recursionDepth = &TConfiguration::DEFAULT_RECURSION_DEPTH;
360+
361+
const TConfiguration config;
362+
BOOST_CHECK_EQUAL(*maxMessageSize, config.getMaxMessageSize());
363+
BOOST_CHECK_EQUAL(*maxFrameSize, config.getMaxFrameSize());
364+
BOOST_CHECK_EQUAL(*recursionDepth, config.getRecursionLimit());
365+
}

0 commit comments

Comments
 (0)