[MDEV-19784]  C++11 protectors for my_error Created: 2019-06-17  Updated: 2023-11-28

Status: In Review
Project: MariaDB Server
Component/s: Compiling
Fix Version/s: 10.11

Type: Task Priority: Minor
Reporter: Nikita Malyavin Assignee: Sergei Golubchik
Resolution: Unresolved Votes: 0
Labels: None

Issue Links:
Blocks
is blocked by MDEV-25015 Custom formatting of strings in Maria... Closed
Relates
relates to MDEV-21978 make my_vsnprintf to use gcc-compatib... Open

 Description   

It is easy to pass wrong arguments in `my_error`: it is vararg, and not printf-like.
One common mistake is passing LEX_CSTRING instead of char*.

1. We can protect from LEX_CSTRING by implementing variadic template guard, forbidding, or implicitly converting some arguments by a sane set of rules, e.g.:

LEX_CSTRING -> char*,
 
anything other -> _error_
 
(TBD)

See also midenok's idea here: https://github.com/tempesta-tech/mariadb/issues/325

It will be still possible to pass int instead of char*, etc.
So, additionally, we can parse a format string and convert an argument to a correct parameter, or throw an assertion.

2. The above approach will reveal errors at runtime, and only for tested cases. We can do it in buildtime, although it will not cover all the cases when error code is determined at runtime:

template<int err_code, class... Args> 
int my_error_guard(Argc... args)
 
{
 
   static_assert(0); // arguments do not fit the prototype for specified error code
 
}
 
 
 
template<class... Args>
 
int my_error_guard<ER_CODE>(char* a, int b)
 
{
 
    my_error(ER_CODE, a, b);    
}
 
 
 
// And so on

When error code is dynamically resolved, it will fall back to dynamic check described above:

#define my_error(code, ...) \
 
        (std::is_const<code>::value \
 
            ? my_error_guard<code>(__VA_ARGS__) \
 
            : my_error(code, ##VA_ARGS))

This implementation will break when const, but not constexpr, error code is passed.



 Comments   
Comment by Sergei Golubchik [ 2020-08-11 ]
  • the list of templates for specific error messages can (should) be generated by comp_err.
  • Ideally we could avoid almost all changes to the sources, by using a macro my_error
Comment by Sergei Golubchik [ 2020-08-11 ]

Another, less C++y option would be to use macros like

#define my_error(X, F, __VA_ARGS__) my_error_ ## X (F, __VA_ARGS__)
 
#define my_error_ER_STD_RANGE_ERROR("Range error: %-.256s in function %s.", F, __VA__ARGS)
 
...

and this way the compiler can see the format string and can verify that parameters are correct

Comment by Nikita Malyavin [ 2020-11-04 ]

serg
>#define my_error(X, F, _VA_ARGS) my_error ## X (F, _VA_ARGS_)
Me and Wlad were also thinking about it.
This way it will stricten the API, for example you couldn't pass error as a non-macro then,
like

my_error(errcode, ....);
 
my_error(flag ? ER_1 : ER_2, ....);

But I think I can generate both codes,

  • one for C++ (which will even support non-consts and dispatch it correctly),
  • and one for plain C (which will stricten the rules, but protect C users as well)
Comment by Nikita Malyavin [ 2020-11-09 ]

Turns out:
1. we can't generate C-compatible code at all. The error storage is highly distributed and allows dynamic updates, which makes it possible to be used in plugins (additional error codes can be added there)
2. I screwed up with std::is_const – it accepts types, not expressions. In general, there's no way to distinguish constexpr from non-constexpr until c++20.This makes impossible to save to use expressions like 

my_error(flag ? ER_1 : ER_2, ....)

If we're forcing compile-time type checking.

serg, (1) still doesn't mean we can't do C-compatible wrappers at all. I see following in service_my_print_error.h:

#ifdef MYSQL_DYNAMIC_PLUGIN
 
 
 
#define my_error my_print_error_service->my_error_func
 
#define my_printf_error my_print_error_service->my_printf_error_func
 
#define my_printv_error(A,B,C,D) my_print_error_service->my_printv_error_func(A,B,C,D)
 
 
 
#else
 
extern void my_error(unsigned int nr, unsigned long MyFlags, ...);
 
extern void my_printf_error(unsigned int my_err, const char *format, unsigned long MyFlags, ...);
 
extern void my_printv_error(unsigned int error, const char *format, unsigned long MyFlags,va_list ap);

So probably my_error is defined separately for plugins, and for non-plugin case we can collect all the messages in one place, namely errmsg-utf8.txt (currently, error messages are stored in several places more in the code).

Comment by Nikita Malyavin [ 2020-11-17 ]

serg please review patches on bb-10.5-nikita regarding this ticket

Generated at Thu Feb 08 08:54:20 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.