Bug #40492 ignored return codes in memory allocation functions

Submitted: 4 Nov 2008 10:51 Modified: 4 Nov 2008 10:52
Reporter: Georgi Kodinov Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: General Severity:S3 (Non-critical)
Version:4.1 and up OS:Any
Assigned to: CPU Architecture:Any

[4 Nov 2008 10:51] Georgi Kodinov
Description:
This is a spin-off of bug #25058 to handle the additional data provided by Mark Callaghan.
Here's a  copy of the data provided in bug #25058:

Warnings from the sql directory. This is the number of ignored return values by called
function.
    814 'bool String::append(const char*, uint32)',  G
    470 'bool String::copy(const char*, uint32, CHARSET_INFO*)',  G
    432 'bool String::real_alloc(uint32)',  G
    382 'bool String::append(const char*)',  G
    229 'bool String::append(char)',  G
    219 'bool String::realloc(uint32)',  G
    126 'bool String::copy(const String&)',  G
     41 'bool MYSQL_LOG::write(Log_event*)',  G
     40 'int end_io_cache(IO_CACHE*)',  G
     40 'char* fn_format(char*, const char*, const char*, const char*, uint)',  G
     38 'my_bool _hash_init(HASH*, CHARSET_INFO*, uint, uint, uint, byte* (*)(const byte*,
uint*, my_bool), void (*)(void*), uint)',  G
     34 'my_bool init_dynamic_array(DYNAMIC_ARRAY*, uint, uint, uint)',  G
     32 'bool String::append(const String&)',  G
     29 'bool MYSQL_LOG::write(THD*, enum_server_command, const char*, ...)',  G
     25 'bool String::append(const char*, uint32, CHARSET_INFO*)',  G
     24 'int my_delete(const char*, myf)',  G
     19 'my_bool my_hash_insert(HASH*, const byte*)',  G
     19 'my_bool insert_dynamic(DYNAMIC_ARRAY*, char*)',  G
     19 'bool get_field(MEM_ROOT*, Field*, String*)',  G
     19 'bool String::copy(const char*, uint32, CHARSET_INFO*, CHARSET_INFO*, uint*)',  G
     19 'bool String::append_with_prefill(const char*, uint32, uint32, char)',  G
     18 'my_bool reinit_io_cache(IO_CACHE*, cache_type, my_off_t, pbool, pbool)',  G
     18 'bool String::alloc(uint32)',  G
     13 'my_off_t my_seek(File, my_off_t, int, myf)',  G
     12 'int quick_rm_table(db_type, const char*, const char*)',  G
     11 'bool String::copy()',  G
     10 'int closefrm(TABLE*)',  G
      5 'char* convert_dirname(char*, const char*, const char*)',  G
      5 'bool remove_table_from_cache(THD*, const char*, const char*, uint)',  G
      5 'bool load_db_opt_by_name(THD*, const char*, HA_CREATE_INFO*)',  G
      4 'int init_ftfuncs(THD*, SELECT_LEX*, bool)',  G
      4 'bool rm_temporary_table(db_type, char*)',  G
      4 'bool close_temporary_table(THD*, const char*, const char*)',  G
      3 'int String::reserve(uint32)',  G
      3 'int MYSQL_LOG::close_index_file()',  G
      3 'bool mysql_rename_table(db_type, const char*, const char*, const char*, const
char*)',  G
      3 'bool MYSQL_LOG::open(const char*, enum_log_type, const char*, cache_type, bool,
ulong, bool)',  G
      2 'int my_rename(const char*, const char*, myf)',  G
      2 'int append_query_string(CHARSET_INFO*, const String*, String*)',  G
      2 'int MYSQL_LOG::purge_logs_before_date(time_t)',  G
      2 'int MYSQL_LOG::get_current_log(LOG_INFO*)',  G
     18 'bool String::alloc(uint32)',  G
     13 'my_off_t my_seek(File, my_off_t, int, myf)',  G
     12 'int quick_rm_table(db_type, const char*, const char*)',  G
     11 'bool String::copy()',  G
     10 'int closefrm(TABLE*)',  G
      5 'char* convert_dirname(char*, const char*, const char*)',  G
      5 'bool remove_table_from_cache(THD*, const char*, const char*, uint)',  G
      5 'bool load_db_opt_by_name(THD*, const char*, HA_CREATE_INFO*)',  G
      4 'int init_ftfuncs(THD*, SELECT_LEX*, bool)',  G
      4 'bool rm_temporary_table(db_type, char*)',  G
      4 'bool close_temporary_table(THD*, const char*, const char*)',  G
      3 'int String::reserve(uint32)',  G
      3 'int MYSQL_LOG::close_index_file()',  G
      3 'bool mysql_rename_table(db_type, const char*, const char*, const char*, const
char*)',  G
      3 'bool MYSQL_LOG::open(const char*, enum_log_type, const char*, cache_type, bool,
ulong, bool)',  G
      2 'int my_rename(const char*, const char*, myf)',  G
      2 'int append_query_string(CHARSET_INFO*, const String*, String*)',  G
      2 'int MYSQL_LOG::purge_logs_before_date(time_t)',  G
      2 'int MYSQL_LOG::get_current_log(LOG_INFO*)',  G
      2 'bool MYSQL_LOG::write(THD*, const char*, uint, time_t, bool)',  G
      1 'uint my_pwrite(File, const byte*, uint, my_off_t, myf)',  G
      1 'my_bool grant_init()',  G
      1 'int my_setwd(const char*, myf)',  G
      1 'int my_realpath(char*, const char*, myf)',  G
      1 'int my_getwd(char*, uint, myf)',  G
      1 'int my_fclose(FILE*, myf)',  G
      1 'int my_chsize(File, my_off_t, int, myf)',  G
      1 'int MYSQL_LOG::raw_get_current_log(LOG_INFO*)',  G
      1 'int MYSQL_LOG::purge_logs(const char*, bool, bool, bool, ulonglong*)',  G
      1 'init_dynamic_array',  G
      1 'bool mysql_rm_db(THD*, char*, bool, bool)',  G
      1 'bool mysql_execute_command(THD*)',  G
      1 'bool mysql_create_db(THD*, char*, HA_CREATE_INFO*, bool)',  G
      1 'bool drop_locked_tables(THD*, const char*, const char*)',  G
      1 'bool dispatch_command(enum_server_command, THD*, char*, uint)',  G
      1 'bool check_access(THD*, ulong, const char*, ulong*, bool, bool, bool)',  G
      1 'bool MYSQL_LOG::open_update_log(const char*)',  G
      1 'bool MYSQL_LOG::open_slow_log(const char*)',  G
      1 'bool MYSQL_LOG::open_query_log(const char*)',  G
      1 'bool MYSQL_LOG::open_audit_log(const char*)',  G
      1 'bool MYSQL_LOG::flush_and_sync()',  G

Counts for ignored return values in the mysys directory
      6 'my_delete',  G
      5 'my_fclose',  G
      3 'my_b_flush_io_cache',  G
      3 'convert_dirname',  G
      2 'my_realpath',  G
      2 'fn_format',  G
      1 'my_rename',  G
      1 'my_readlink',  G
      1 'my_fseek',  G
      1 'fn_same',  G
      1 'end_io_cache',  G

And for the myisam directory. For all of these, I changed sql_class.h, my_sys.h,
mysql_priv.h, sql_acl.h, sql_string.h and hash.h. Many headers were not edited, so there
are many ignored return values not reported.

     23 'fn_format',  G
     12 'end_io_cache',  G
      9 'my_delete',  G
      7 'my_write',  G
      7 'my_seek',  G
      6 'reinit_io_cache',  G
      5 'my_lock',  G
      2 'my_delete_with_symlink',  G
      2 'insert_dynamic',  G
      2 'init_io_cache',  G
      1 'my_realpath',  G
      1 'my_fclose',  G
      1 'my_copystat',  G
      1 'my_chsize',  G
      1 'init_dynamic_array',  G

How to repeat:
Look at the code

Suggested fix:
use warn_unused_result to identify and then fix all the above places.
[4 May 2010 17:00] Mark Callaghan
For 5.1.46, I count 2500+ warnings for ignored return values from the methods in sql_string.h that fail when memory allocation fails.
[26 May 2010 11:51] Konstantin Osipov
The reason risk is high is that some places where we ignore return codes require a change in the execution flow, i.e., since we don't use RAII (http://en.wikipedia.org/wiki/RAII), addition of end: labels with resource cleanup of every function involved. 
Subtle bugs can easily get introduced when adding a jump to a label, not all resources can get cleaned upon error, etc.

High effort since the real fix is to not go over the code and add a bunch of checks, but make sure that sloppy code is not re-introduced. 
This requires a framework for testing the server in an environment that has spurious OOM errors.
[26 May 2010 14:04] James Day
Is there any work here that is low risk? I'm thinking that a developer could decide which ones are low risk and do those as a first step, then defer anything else. It won't be perfect but it'll reduce the risk of encountering uncaught failed memory allocations at runtime a bit. Gradual improvement beats none in things like this.

Portions of this website are copyright © 2001, 2002 The PHP Group

Page generated in 0.065 sec. using MySQL 8.4.7-u6-cloud

Timestamp references displayed by the system are UTC.
Content reproduced on this site is the property of the respective copyright holders.
It is not reviewed in advance by Oracle and does not necessarily represent the opinion of Oracle or any other party.