Skip to content
/ server Public

Commit 7845761

Browse files
ParadoxV5vuvovabnestere
committed
MDEV-37530 fixes
* MDEV-38410: Use array, not `std::initializer_list` Some environments appear not to retain the backing array of a static `std::initializer_list` in the MDEV-37530 release candidate, and eventually crash when reading overwritten data. This commit resolves the stealth issue by reverting to conventional arrays, while maintaining convenience through deductive overloads. * Compile problems * Some of our platforms (namely SUSE 15, which uses GCC 7.5) support C++17 syntaxes, but not all libraries, `<charconv>`` among those. * Update to the current `main` branch Co-authored-by: Sergei Golubchik <[email protected]> Co-authored-by: Brandon Nesterenko <[email protected]>
1 parent 8f8bf21 commit 7845761

File tree

9 files changed

+105
-119
lines changed

9 files changed

+105
-119
lines changed

mysql-test/include/rpl_heartbeat.inc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22
# Shared between rpl.rpl_heartbeat and binlog_in_engine.rpl_heartbeat.
33
#
44
# Including:
5-
# - user interface, grammar, checking the range and warnings about
6-
# unreasonable values for the heartbeat period;
75
# - no rotation of relay log if heartbeat is less that slave_net_timeout
86
# - SHOW STATUS like 'Slave_received_heartbeats' action
97
# - SHOW STATUS like 'Slave_heartbeat_period' report
8+
#
9+
# `rpl.rpl_heartbeat_basic` tests the user interface, grammar, range,
10+
# and warnings about unreasonable values for the heartbeat period.
1011

1112
connection slave;
1213
-- source include/stop_slave.inc

mysql-test/main/change_master_default.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ master_ssl_ca_path,
88
master_ssl_cert,
99
master_ssl_cipher,
1010
master_ssl_key,
11-
`master_ssl_verify_server_cert`, # MDEV-38194
11+
master_ssl_verify_server_cert,
1212
master_ssl_crl,
1313
master_ssl_crlpath,
1414
using_gtid,

mysql-test/main/change_master_default.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ SELECT connection_name,
2020
master_ssl_cert,
2121
master_ssl_cipher,
2222
master_ssl_key,
23-
`master_ssl_verify_server_cert`, # MDEV-38194
23+
master_ssl_verify_server_cert,
2424
master_ssl_crl,
2525
master_ssl_crlpath,
2626
using_gtid,

mysql-test/suite/binlog_in_engine/rpl_heartbeat.result

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,41 +8,6 @@ reset master;
88
connection slave;
99
set @restore_slave_net_timeout= @@global.slave_net_timeout;
1010
set @@global.slave_net_timeout= 10;
11-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root';
12-
show status like 'Slave_heartbeat_period';;
13-
Variable_name Slave_heartbeat_period
14-
Value 5.000
15-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4294968;
16-
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
17-
show status like 'Slave_heartbeat_period';;
18-
Variable_name Slave_heartbeat_period
19-
Value 5.000
20-
connection slave;
21-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.0009999;
22-
Warnings:
23-
Warning 1703 The requested value for the heartbeat period is less than 1 millisecond. The value is reset to 0, meaning that heartbeating will effectively be disabled
24-
show status like 'Slave_heartbeat_period';;
25-
Variable_name Slave_heartbeat_period
26-
Value 0.000
27-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4294967;
28-
Warnings:
29-
Warning 1704 The requested value for the heartbeat period exceeds the value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout
30-
show status like 'Slave_heartbeat_period';;
31-
Variable_name Slave_heartbeat_period
32-
Value 4294967.000
33-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.001;
34-
show status like 'Slave_heartbeat_period';;
35-
Variable_name Slave_heartbeat_period
36-
Value 0.001
37-
reset slave;
38-
set @@global.slave_net_timeout= 5;
39-
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 5.001;
40-
Warnings:
41-
Warning 1704 The requested value for the heartbeat period exceeds the value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout
42-
show status like 'Slave_heartbeat_period';;
43-
Variable_name Slave_heartbeat_period
44-
Value 5.001
45-
reset slave;
4611
set @@global.slave_net_timeout= 5;
4712
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4;
4813
show status like 'Slave_heartbeat_period';;

mysql-test/suite/rpl/r/rpl_heartbeat_basic.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
162162
Variable_name Value
163163
Slave_heartbeat_period 4294967.295
164164
RESET SLAVE;
165-
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294968;
165+
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967.296;
166166
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967.295 seconds)
167167
RESET SLAVE;
168168
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=8589935;

mysql-test/suite/rpl/t/rpl_heartbeat_basic.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
210210
RESET SLAVE;
211211
--replace_result $MASTER_MYPORT MASTER_PORT
212212
--error ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
213-
eval CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=$MASTER_MYPORT, MASTER_USER='root', MASTER_CONNECT_RETRY=$connect_retry, MASTER_HEARTBEAT_PERIOD=4294968;
213+
eval CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=$MASTER_MYPORT, MASTER_USER='root', MASTER_CONNECT_RETRY=$connect_retry, MASTER_HEARTBEAT_PERIOD=4294967.296;
214214
RESET SLAVE;
215215
# Check double size of max allowed value for master_heartbeat_period
216216
--replace_result $MASTER_MYPORT MASTER_PORT

sql/rpl_info_file.h

Lines changed: 71 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#define RPL_INFO_FILE_H
2020

2121
#include <cstdint> // uintN_t
22-
#include <charconv> // std::from/to_chars and other helpers
2322
#include <functional> // superclass of Info_file::Mem_fn
2423
#include <my_sys.h> // IO_CACHE, FN_REFLEN, ...
2524

@@ -36,7 +35,6 @@ namespace Int_IO_CACHE
3635
*/
3736
template<typename I> static constexpr size_t BUF_SIZE=
3837
std::numeric_limits<I>::digits10 + 1 + std::numeric_limits<I>::is_signed;
39-
static constexpr auto ERRC_OK= std::errc();
4038

4139
/**
4240
@ref IO_CACHE (reading one line with the `\n`) version of std::from_chars()
@@ -45,19 +43,38 @@ namespace Int_IO_CACHE
4543
*/
4644
template<typename I> static bool from_chars(IO_CACHE *file, I &value)
4745
{
46+
int error;
4847
/**
49-
+2 for the terminating `\n\0` (They are ignored by
50-
std::from_chars(), but my_b_gets() includes them.)
48+
+2 for the terminating `\n\0`
49+
(They are ignored, but my_b_gets() includes them.)
5150
*/
5251
char buf[BUF_SIZE<I> + 2];
5352
/// includes the `\n` but excludes the `\0`
5453
size_t length= my_b_gets(file, buf, sizeof(buf));
5554
if (!length) // EOF
5655
return true;
57-
// SFINAE if `I` is not a numeric type
58-
std::from_chars_result result= std::from_chars(buf, &(buf[length]), value);
59-
// Return `true` if the conversion failed or if the number ended early
60-
return result.ec != ERRC_OK || *(result.ptr) != '\n';
56+
char *end= &(buf[length]);
57+
longlong val= my_strtoll10(buf, &end, &error);
58+
switch (error) {
59+
case -1:
60+
if (!std::numeric_limits<I>::is_signed)
61+
return true;
62+
[[fallthrough]];
63+
case 0:
64+
/*TODO
65+
This upper range check is not needed when using type-
66+
specific variants of a safe string-to-integer converter
67+
(e.g., std::from_chars() when all platforms support it).
68+
*/
69+
if (*end == '\n' && value <= std::numeric_limits<I>::max())
70+
{
71+
value= static_cast<I>(val);
72+
return false;
73+
}
74+
[[fallthrough]];
75+
default:
76+
return true;
77+
}
6178
}
6279
/**
6380
Convenience overload of from_chars(IO_CACHE *, I &) for `operator=` types
@@ -79,14 +96,19 @@ namespace Int_IO_CACHE
7996
*/
8097
template<typename I> static void to_chars(IO_CACHE *file, I value)
8198
{
82-
/**
83-
my_b_printf() uses a buffer too,
84-
so we might as well save on format parsing and buffer resizing
85-
*/
8699
char buf[BUF_SIZE<I>];
87-
std::to_chars_result result= std::to_chars(buf, &buf[sizeof(buf)], value);
88-
DBUG_ASSERT(result.ec == ERRC_OK);
89-
my_b_write(file, reinterpret_cast<const uchar *>(buf), result.ptr - buf);
100+
/*TODO:
101+
* my_b_printf() needs updates and so doesn't
102+
support `long long`s at the moment.
103+
* We can avoid format parsing by expanding
104+
int10_to_str() if not supporting std::to_chars().
105+
*/
106+
int len= std::numeric_limits<I>::is_signed ?
107+
snprintf(buf, BUF_SIZE<I>, "%lld", static_cast<long long>(value)) :
108+
snprintf(buf, BUF_SIZE<I>, "%llu", static_cast<unsigned long long>(value))
109+
;
110+
DBUG_ASSERT(len > 0);
111+
my_b_write(file, reinterpret_cast<const uchar *>(buf), len);
90112
}
91113
};
92114

@@ -209,7 +231,6 @@ struct Info_file
209231
*/
210232
struct Mem_fn: std::function<Persistent &(Info_file *self)>
211233
{
212-
using List= const std::initializer_list<Mem_fn>;
213234
/// Null Constructor
214235
Mem_fn(std::nullptr_t null= nullptr):
215236
std::function<Persistent &(Info_file *)>(null) {}
@@ -242,36 +263,55 @@ struct Info_file
242263
(either contains a `.` or is entirely empty) rather than an integer.
243264
@return `false` if the file has parsed successfully or `true` if error
244265
*/
245-
bool load_from_file(Mem_fn::List value_list, size_t default_line_count)
266+
template<size_t size> bool load_from_file(
267+
const Mem_fn (&value_list)[size],
268+
size_t default_line_count= 0
269+
) { return load_from_file(value_list, size, default_line_count); }
270+
/**
271+
Flush the MySQL line-based section to the @ref file
272+
@param value_list List of wrapped member pointers to values.
273+
@param total_line_count
274+
The number of lines to describe the file as on the first line of the file.
275+
If this is larger than `value_list.size()`, suffix the file with empty
276+
lines until the line count (including the line count line) is this many.
277+
This reservation provides compatibility with MySQL,
278+
who has added more old-style lines while MariaDB innovated.
279+
*/
280+
template<size_t size> void save_to_file(
281+
const Mem_fn (&value_list)[size],
282+
size_t total_line_count= size + /* line count line */ 1
283+
) { return save_to_file(value_list, size, total_line_count); }
284+
285+
private:
286+
bool
287+
load_from_file(const Mem_fn *values, size_t size, size_t default_line_count)
246288
{
289+
long val;
247290
/**
248291
The first row is temporarily stored in the first value. If it is a line
249292
count and not a log name (new format), the second row will overwrite it.
250293
*/
251-
auto &line1= dynamic_cast<String_value<> &>((*(value_list.begin()))(this));
294+
auto &line1= dynamic_cast<String_value<> &>(values[0](this));
252295
if (line1.load_from(&file))
253296
return true;
254-
size_t line_count;
255-
std::from_chars_result result= std::from_chars(
256-
line1.buf, &line1.buf[sizeof(line1.buf)], line_count);
297+
char *end= str2int(line1.buf, 10, 0, INT32_MAX, &val);
257298
/**
258299
If this first line was not a number - the line count,
259300
then it was the first value for real,
260301
so the for loop should then skip over it, the index 0 of the list.
261302
*/
262-
size_t i= result.ec != Int_IO_CACHE::ERRC_OK || *(result.ptr) != '\0';
303+
size_t i= !end || *end != '\0';
263304
/*
264305
Set the default after parsing: While std::from_chars() does not replace
265306
the output if it failed, it does replace if the line is not fully spent.
266307
*/
267-
if (i)
268-
line_count= default_line_count;
308+
size_t line_count= i ? default_line_count: static_cast<size_t>(val);
269309
for (; i < line_count; ++i)
270310
{
271311
int c;
272-
if (i < value_list.size()) // line known in the `value_list`
312+
if (i < size) // line known in the `value_list`
273313
{
274-
const Mem_fn &pm= value_list.begin()[i];
314+
const Mem_fn &pm= values[i];
275315
if (pm)
276316
{
277317
if (pm(this).load_from(&file))
@@ -294,19 +334,9 @@ struct Info_file
294334
return false;
295335
}
296336

297-
/**
298-
Flush the MySQL line-based section to the @ref file
299-
@param value_list List of wrapped member pointers to values.
300-
@param total_line_count
301-
The number of lines to describe the file as on the first line of the file.
302-
If this is larger than `value_list.size()`, suffix the file with empty
303-
lines until the line count (including the line count line) is this many.
304-
This reservation provides compatibility with MySQL,
305-
who has added more old-style lines while MariaDB innovated.
306-
*/
307-
void save_to_file(Mem_fn::List value_list, size_t total_line_count)
337+
void save_to_file(const Mem_fn *values, size_t size, size_t total_line_count)
308338
{
309-
DBUG_ASSERT(total_line_count > value_list.size());
339+
DBUG_ASSERT(total_line_count > size);
310340
my_b_seek(&file, 0);
311341
/*
312342
If the new contents take less space than the previous file contents,
@@ -316,8 +346,9 @@ struct Info_file
316346
*/
317347
Int_IO_CACHE::to_chars(&file, total_line_count);
318348
my_b_write_byte(&file, '\n');
319-
for (const Mem_fn &pm: value_list)
349+
for (size_t i= 0; i < size; ++i)
320350
{
351+
const Mem_fn &pm= values[i];
321352
if (pm)
322353
pm(this).save_to(&file);
323354
my_b_write_byte(&file, '\n');
@@ -327,7 +358,7 @@ struct Info_file
327358
(1 for the line count line + line count) inclusive -> max line inclusive
328359
= line count exclusive <- max line inclusive
329360
*/
330-
for (; total_line_count > value_list.size(); --total_line_count)
361+
for (; total_line_count > size; --total_line_count)
331362
my_b_write_byte(&file, '\n');
332363
}
333364

0 commit comments

Comments
 (0)