Skip to content
/ server Public

MDEV-39138: Fix ER_OUTOFMEMORY to use %zu (10.6)#4850

Open
Harsh8084 wants to merge 1 commit intoMariaDB:10.6from
Harsh8084:fix-MDEV-39138-10.6
Open

MDEV-39138: Fix ER_OUTOFMEMORY to use %zu (10.6)#4850
Harsh8084 wants to merge 1 commit intoMariaDB:10.6from
Harsh8084:fix-MDEV-39138-10.6

Conversation

@Harsh8084
Copy link
Copy Markdown

Backport of MDEV-39138 fix to 10.6.

This patch:

  • Replaces incorrect %d with %zu for size_t
  • Fixes unsafe casting of negative error codes to size_t
  • Removes undefined variable usage (buf_len)
  • Corrects duplicate JSON append logic
  • Improves error handling in JSON functions

Notes:

  • Previous behavior incorrectly cast negative values (e.g., -6) to unsigned, leading to large incorrect values
  • Updated test results accordingly where behavior relied on incorrect formatting

Re-recorded affected tests including main.perror.

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 24, 2026
Copy link
Copy Markdown
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! This is a preliminary review.

This pull request looks like some AI tool has been used without actually reviewing the outcome. Note that work done by AIs is not copyrightable. Are you sure you can legally sign it off as you did?

Please always do a self-review prior to submitting a pull request.

Please also make sure you also add a commit message that complies with the MariaDB coding standards to your commit.

return 0;
}

str_len= json_unescape(json_cs, js, js + js_len, s->charset(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do json_unescape twice!

if (str_len >= 0)
return 0;

if (current_thd)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's always current_thd! Please turn this into an assert?

@@ -805,6 +823,15 @@ String *Item_func_json_unquote::val_str(String *str)

error:
report_json_error(js, &je, 0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

report_json_error already reports an error. Why do you need to duplicate this? I'd rather extend report_json_error() to include the length and extend it to report out of memory.

(n_item == 0 && str->append(" ", 1)) ||
append_simple(str, item_pos, js->end() - item_pos))
goto return_null; /* Out of memory. */
my_ptrdiff_t size= item_pos - js->ptr();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either something is wrong with the diff or you've inadvertently left the original you're trying to change too.
Instead of unfolding the if() just please add a warning in its then branch.

append_json_value(str, args[n_arg+1], &tmp_val) ||
append_simple(str, item_pos, js->end() - item_pos))
goto return_null; /* Out of memory. */
size= item_pos - js->ptr();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here: see the other comment.

{
if (value2.realloc_with_extra_if_needed(je.value_len) ||
(c_len= json_unescape(js->charset(), je.value,
if (value2.realloc_with_extra_if_needed(je.value_len))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you've again left the original call.

if (gtid_str.alloc(WSREP_MAX_WSREP_SERVER_GTID_STR_LEN+1))
{
my_error(ER_OUTOFMEMORY, MYF(0), WSREP_MAX_WSREP_SERVER_GTID_STR_LEN);
my_error(ER_OUTOFMEMORY, MYF(0), (size_t)WSREP_MAX_WSREP_SERVER_GTID_STR_LEN);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? revert the whole file please.

DBUG_EXECUTE_IF("dbug_session_tracker_parse_error",
{
my_error(ER_OUTOFMEMORY, MYF(0), 1);
my_error(ER_OUTOFMEMORY, MYF(0), (size_t)1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert the whole file please

if (!res)
{
my_error(ER_OUTOFMEMORY, MYF(0), (int)((count+1)*sizeof(*res)));
my_error(ER_OUTOFMEMORY, MYF(0), ((count+1)*sizeof(*res)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert the whole file please.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not change submodules!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

2 participants