* Davi Arnaut <Davi.Arnaut@stripped> [09/01/30 15:53]:
> 2715 Davi Arnaut 2009-01-30
> Remove the global errbuf as this buffer is not needed anymore and
> its use can lead to problems in a threaded environment.
OK to push, please see comments below.
> modified:
> include/my_global.h
> include/my_sys.h
> mysys/my_error.c
> mysys/my_init.c
> mysys/safemalloc.c
> sql/backup/kernel.cc
> sql/backup/logger.cc
> sql/sql_error.cc
> sql/sql_parse.cc
> sql/sql_table.cc
> storage/maria/ha_maria.cc
> storage/myisam/ha_myisam.cc
>
> === modified file 'include/my_sys.h'
> --- a/include/my_sys.h 2009-01-16 11:53:32 +0000
> +++ b/include/my_sys.h 2009-01-30 12:32:09 +0000
> @@ -43,8 +43,6 @@ extern int NEAR my_errno; /* Last error
> #define MYSYS_PROGRAM_DONT_USE_CURSES() { error_handler_hook =
> my_message_no_curses; mysys_uses_curses=0;}
> #define MY_INIT(name); { my_progname= name; my_init(); }
>
> -#define ERRMSGSIZE (SC_MAXWIDTH) /* Max length of a error message */
> -#define NRERRBUFFS (2) /* Buffers for parameters */
Use in my_init.c suggests (to me) we should keep it.
Please consider leaving the declaration here, but rename it to
MYSYS_ERRMSG_SIZE, and the following comment:
/**
Max length of an error message generated by mysys utilities.
Some mysys functions produce error messages. These mostly go
to stderr.
This constant defines the size of the buffer used to format
the message. It should be kept in sync with MYSQL_ERRMSG_SIZE,
since sometimes mysys errors are stored in the server diagnostics
area, and we would like to avoid unexpected truncation.
*/
> #define MY_FILE_ERROR ((size_t) -1)
>
> /* General bitmaps for my_func's */
> @@ -214,7 +212,6 @@ extern void my_large_free(uchar * ptr, m
> extern int errno; /* declare errno */
> #endif
> #endif /* #ifndef errno */
> -extern char NEAR errbuff[NRERRBUFFS][ERRMSGSIZE];
Good.
> - sprintf(errbuff[0],EE(EE_OPEN_WARNING),my_file_opened,my_stream_opened);
> - (void) my_message_no_curses(EE_OPEN_WARNING,errbuff[0],ME_BELL);
> - DBUG_PRINT("error",("%s",errbuff[0]));
> + char ebuff[512];
> + my_snprintf(ebuff, sizeof(ebuff), EE(EE_OPEN_WARNING),
> + my_file_opened, my_stream_opened);
> + my_message_no_curses(EE_OPEN_WARNING, ebuff, ME_BELL);
> + DBUG_PRINT("error", ("%s", ebuff));
> my_print_open_files();
> }
> }
Suggest to use MYSYS_ERRMSG_SIZE
> === modified file 'mysys/safemalloc.c'
> --- a/mysys/safemalloc.c 2008-10-20 09:16:47 +0000
> +++ b/mysys/safemalloc.c 2009-01-30 12:32:09 +0000
> @@ -147,7 +147,7 @@ void *_mymalloc(size_t size, const char
> error_handler_hook=fatal_error_handler_hook;
> if (MyFlags & (MY_FAE+MY_WME))
> {
> - char buff[SC_MAXWIDTH];
> + char buff[256];
Suggest to use MYSYS_ERRMSG_SIZE and my_snprintf insteadof sprintf
> my_errno=errno;
> sprintf(buff,"Out of memory at line %d, '%s'", lineno, filename);
> my_message(EE_OUTOFMEMORY, buff, MYF(ME_BELL+ME_WAITTANG+ME_NOREFRESH));
>
> === modified file 'sql/backup/kernel.cc'
> --- a/sql/backup/kernel.cc 2009-01-14 10:10:00 +0000
> +++ b/sql/backup/kernel.cc 2009-01-30 12:32:09 +0000
> @@ -266,7 +266,7 @@ int send_error(Backup_restore_ctx &conte
> {
> if (!context.error_reported())
> {
> - char buf[ERRMSGSIZE + 20];
> + char buf[MYSQL_ERRMSG_SIZE];
> va_list args;
Good.
> va_start(args, error_code);
>
>
> === modified file 'sql/backup/logger.cc'
> --- a/sql/backup/logger.cc 2008-12-10 15:53:06 +0000
> +++ b/sql/backup/logger.cc 2009-01-30 12:32:09 +0000
> @@ -33,7 +33,7 @@ namespace backup {
> int Logger::write_message(log_level::value level, int error_code,
> const char *msg)
> {
> - char buf[ERRMSGSIZE + 30];
> + char buf[MYSQL_ERRMSG_SIZE];
Excellent.
> /*
> When logging to server's error log, msg will be prefixed with
> "Backup:"/"Restore:" if the operation has been initialized (i.e., after
> @@ -141,7 +141,7 @@ int Logger::v_report_error(log_level::va
> int Logger::v_write_message(log_level::value level, int error_code,
> const char *format, va_list args)
> {
> - char buf[ERRMSGSIZE + 20];
> + char buf[MYSQL_ERRMSG_SIZE];
Good.
>
> my_vsnprintf(buf, sizeof(buf), format, args);
> return write_message(level, error_code, buf);
>
All other sql/ parts are good too.
> === modified file 'storage/maria/ha_maria.cc'
> --- a/storage/maria/ha_maria.cc 2009-01-13 15:26:20 +0000
> +++ b/storage/maria/ha_maria.cc 2009-01-30 12:32:09 +0000
> @@ -1457,7 +1457,7 @@ int ha_maria::preload_keys(THD * thd, HA
>
> if ((error= maria_preload(file, map, table_list->ignore_leaves)))
> {
> - char buf[ERRMSGSIZE+20];
> + char buf[128];
> const char *errmsg;
Hm... I guess maria needs an own constant. Please keep
MYSYS_ERRMSG_SIZE here for now, but without the ugly +20 (what do
they mean anyway? Perhaps they are here from
before-the-massive-replace-of-sprintf-with-my_snprintf times).
>
> switch (error) {
> @@ -1468,7 +1468,7 @@ int ha_maria::preload_keys(THD * thd, HA
> errmsg= "Failed to allocate buffer";
> break;
> default:
> - my_snprintf(buf, ERRMSGSIZE,
> + my_snprintf(buf, sizeof(buf),
> "Failed to read from index file (errno: %d)", my_errno);
> errmsg= buf;
> }
>
> === modified file 'storage/myisam/ha_myisam.cc'
> --- a/storage/myisam/ha_myisam.cc 2009-01-19 13:16:25 +0000
> +++ b/storage/myisam/ha_myisam.cc 2009-01-30 12:32:09 +0000
> @@ -1134,7 +1134,7 @@ int ha_myisam::preload_keys(THD* thd, HA
> ulonglong map;
> TABLE_LIST *table_list= table->pos_in_table_list;
> my_bool ignore_leaves= table_list->ignore_leaves;
> - char buf[ERRMSGSIZE+20];
> + char buf[MYSQL_ERRMSG_SIZE];
>
> DBUG_ENTER("ha_myisam::preload_keys");
>
> @@ -1162,7 +1162,7 @@ int ha_myisam::preload_keys(THD* thd, HA
> errmsg= "Failed to allocate buffer";
> break;
> default:
> - my_snprintf(buf, ERRMSGSIZE,
> + my_snprintf(buf, sizeof(buf),
> "Failed to read from index file (errno: %d)", my_errno);
> errmsg= buf;
> }
>
--