maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Serg,

Ok the patch moving in the right direction but it's not there, yet.

Please find the input below.

First, input on the commit comment. Please consider it as a request applying
to ALL further commits to the MariaDB codebase.


> MDEV-23766: implemented requested debug checks

Imagine somebody looking at this in a few years. Will they know what checks
were requested? They might get a clue by looking at the Jira text, but the
idea to make the commit comments concise and self-contained descriptions.

Something like this:

Line #1: one-line description of the patch:

>> MDEV-23766: Make attempts to write invalid JSON cause assertion failures

Subsequent lines:

>> Make JSON writing API (class Json_writer) check the produced JSON document
>> is valid. The following checks are made:
>> - JSON objects must contain named members
>> - JSON arrays must contain unnamed members.
>> - (TODO: add other restrictions we're enforcing).


It is not clear how the checker interacts with Single_line_formatting_helper.
The code like one the one I'm quoting below shows some non-trivial overlap:

> @@ -28,7 +36,16 @@ void Json_writer::append_indent()
>  
>  void Json_writer::start_object()
>  {
> +#ifndef NDEBUG
> +  DBUG_ASSERT(got_name == named_item_expected());
> +  named_items_expectation.push_back(true);
> +  size_t depth= named_items_expectation.size();
> +#endif
>    fmt_helper.on_start_object();
> +#ifndef NDEBUG
> +  if (depth != named_items_expectation.size())
> +    named_items_expectation.pop_back();
> +#endif
>  
>    if (!element_started)
>      start_element();

This looks very cryptic. Can you elaborate about this?

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net