Hi, Oleksandr! On Mar 22, Oleksandr Byelkin wrote:

revision-id: 4b142536a24 (mariadb-10.2.31-56-g4b142536a24) parent(s): ed21202a14e author: Oleksandr Byelkin <sanja@mariadb.com> committer: Oleksandr Byelkin <sanja@mariadb.com> timestamp: 2020-03-18 08:25:13 +0100 message:

MDEV-20604: Duplicate key value is silently truncated to 64 characters in print_keydup_error

Added indication of truncated string with special width/precission suffix ':'.

The comment looks wrong :)

diff --git a/client/mysql_upgrade.c b/client/mysql_upgrade.c index 0bbb8a149ff..003701514cf 100644 --- a/client/mysql_upgrade.c +++ b/client/mysql_upgrade.c @@ -501,10 +499,10 @@ static void find_tool(char *tool_executable_name, const char *tool_name, last_fn_libchar -= 6; }

- len= (int)(last_fn_libchar - self_name); - - my_snprintf(tool_executable_name, FN_REFLEN, "%.*s%c%s", - len, self_name, FN_LIBCHAR, tool_name); + last_fn_libchar[0]= 0; + my_snprintf(tool_executable_name, FN_REFLEN, "%s%c%s", + self_name, FN_LIBCHAR, tool_name); + last_fn_libchar[0]= FN_LIBCHAR;

Okay, but why not to use %b here?

}

if (opt_verbose) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 60a203ccedd..045ab566fdb 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -9534,6 +9533,7 @@ int main(int argc, char **argv) case Q_LET: do_let(command); break; case Q_EVAL_RESULT: die("'eval_result' command is deprecated"); + break; // never called but keep compiler calm

wouldn't it be better to specify __attribute__ ((noreturn)) for die()?

case Q_EVAL: case Q_EVALP: case Q_QUERY_VERTICAL: diff --git a/strings/my_vsnprintf.c b/strings/my_vsnprintf.c index ad3517e4252..9ae10f337cc 100644 --- a/strings/my_vsnprintf.c +++ b/strings/my_vsnprintf.c @@ -28,7 +28,9 @@ #define LENGTH_ARG 1 #define WIDTH_ARG 2 #define PREZERO_ARG 4 -#define ESCAPED_ARG 8 +#define ESCAPED_ARG 8 +// Show truncation of string argument +#define SH_TRUNC_ARG 16

you don't really need a flag, because process_str_arg should always do it. By the way, in your code it doesn't. It doesn't print dots for %M and for %1$s.

typedef struct pos_arg_info ARGS_INFO; typedef struct print_info PRINT_INFO; @@ -198,18 +199,42 @@ static char *process_str_arg(CHARSET_INFO *cs, char *to, const char *end, size_t width, char *par, uint print_type) { int well_formed_error; - size_t plen, left_len= (size_t) (end - to) + 1; + uint dots= 0; + size_t plen, left_len= (size_t) (end - to) + 1, slen=0; if (!par) par = (char*) "(null)";

- plen= strnlen(par, width); + plen= slen= strnlen(par, width + 1); + if (plen > width) + plen= width; if (left_len <= plen) plen = left_len - 1; + if ((slen > plen) && (print_type & SH_TRUNC_ARG)) + { + if (plen < 3) + { + dots= plen; + plen= 0; + } + else + { + plen-= 3; + dots= 3; + } + } + plen= my_well_formed_length(cs, par, par + plen, width, &well_formed_error); if (print_type & ESCAPED_ARG) to= backtick_string(cs, to, end, par, plen, '`'); else to= strnmov(to,par,plen); + + if (dots) + { + for (; dots; dots--) + *(to++)= '.'; + *(to)= 0; + }

This should look strange when truncating a quoted string. See the test case below

return to; }

diff --git a/unittest/mysys/my_vsnprintf-t.c b/unittest/mysys/my_vsnprintf-t.c index 6ba0a42cf7e..e8831f7c1f4 100644 --- a/unittest/mysys/my_vsnprintf-t.c +++ b/unittest/mysys/my_vsnprintf-t.c @@ -99,12 +99,12 @@ int main(void) test1("Width is ignored for strings <x> <y>", "Width is ignored for strings <%04s> <%5s>", "x", "y");

- test1("Precision works for strings <abcde>", + test1("Precision works for strings <ab...>", "Precision works for strings <%.5s>", "abcdef!");

- test1("Flag '`' (backtick) works: `abcd` `op``q` (mysql extension)", This - "Flag '`' (backtick) works: %`s %`.4s (mysql extension)", This - "abcd", "op`qrst"); + test1("Flag '`' (backtick) works: `abcd` `op``q`... (mysql extension)", + "Flag '`' (backtick) works: %`s %`.7s (mysql extension)", + "abcd", "op`qrstuuuuuuuuu");

Here, the test case with a quoted string. First, you print 10 characters instead of 7. Second, you put dots after the closing backtick, which looks confusing, the truncation happens before the backtick. Third, as you can see, you cannot use strlen() for a string length because there can be backticks inside the string. perhaps it would be simpler to move ... inside backtick_string()

test1("Length modifiers work: 1 * -1 * 2 * 3", "Length modifiers work: %d * %ld * %lld * %zd", 1, -1L, 2LL, (size_t)3); @@ -125,7 +125,7 @@ int main(void) test1("Asterisk '*' as a width works: < 4>", "Asterisk '*' as a width works: <%*d>", 5, 4);

- test1("Asterisk '*' as a precision works: <qwerty>", + test1("Asterisk '*' as a precision works: <qwe...>", "Asterisk '*' as a precision works: <%.*s>", 6, "qwertyuiop");

please add tests for ...truncation in %M and %1$s

test1("Positional arguments for a width: < 4>",

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org