MDEV-39138: Fix ER_OUTOFMEMORY to use %zu (10.6)#4850
MDEV-39138: Fix ER_OUTOFMEMORY to use %zu (10.6)#4850Harsh8084 wants to merge 1 commit intoMariaDB:10.6from
Conversation
gkodinov
left a comment
There was a problem hiding this comment.
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(), |
There was a problem hiding this comment.
No need to do json_unescape twice!
| if (str_len >= 0) | ||
| return 0; | ||
|
|
||
| if (current_thd) |
There was a problem hiding this comment.
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); | |||
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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); |
| if (!res) | ||
| { | ||
| my_error(ER_OUTOFMEMORY, MYF(0), (int)((count+1)*sizeof(*res))); | ||
| my_error(ER_OUTOFMEMORY, MYF(0), ((count+1)*sizeof(*res))); |
Backport of MDEV-39138 fix to 10.6.
This patch:
Notes:
Re-recorded affected tests including main.perror.