← Back to team overview

maria-developers team mailing list archive

Re: [Commits] ab7640f: my_b_fill, inline my_b_* functions instead of hairy macros

 

Hi Sergei,

ok to push. Should we always prefer "static inline" over macroses?

Regards,
Sergey

On Tue, May 26, 2015 at 03:38:02PM +0200, serg@xxxxxxxxxxx wrote:
> revision-id: ab7640f4d30f5a160d3a1c77efe9d2c459a3bda6
> parent(s): d916a1dc7705ae29a916e7ec91f0c76bceab708c
> committer: Sergei Golubchik
> branch nick: maria
> timestamp: 2015-05-26 15:28:23 +0200
> message:
> 
> my_b_fill, inline my_b_* functions instead of hairy macros
> 
> ---
>  include/my_sys.h    | 141 ++++++++++++++++++++++++++++++++++++----------------
>  include/thr_lock.h  |   1 -
>  mysys/mf_iocache2.c |  55 --------------------
>  sql/log.cc          |   1 -
>  sql/wsrep_binlog.cc |   4 --
>  5 files changed, 97 insertions(+), 105 deletions(-)
> 
> diff --git a/include/my_sys.h b/include/my_sys.h
> index 3d930d3..8e0e34f 100644
> --- a/include/my_sys.h
> +++ b/include/my_sys.h
> @@ -19,6 +19,8 @@
>  
>  #include "my_global.h"                  /* C_MODE_START, C_MODE_END */
>  
> +#include <m_string.h>
> +
>  C_MODE_START
>  
>  #ifdef HAVE_AIOWAIT
> @@ -507,52 +509,108 @@ typedef void (*my_error_reporter)(enum loglevel level, const char *format, ...)
>  
>  extern my_error_reporter my_charset_error_reporter;
>  
> -	/* defines for mf_iocache */
> +/* inline functions for mf_iocache */
> +
> +extern int my_b_flush_io_cache(IO_CACHE *info, int need_append_buffer_lock);
> +extern int _my_b_get(IO_CACHE *info);
> +extern int _my_b_read(IO_CACHE *info,uchar *Buffer,size_t Count);
> +extern int _my_b_write(IO_CACHE *info,const uchar *Buffer,size_t Count);
>  
> -	/* Test if buffer is inited */
> -#define my_b_clear(info) (info)->buffer=0
> -#define my_b_inited(info) (info)->buffer
> +/* Test if buffer is inited */
> +static inline void my_b_clear(IO_CACHE *info) { info->buffer= 0; }
> +static inline int my_b_inited(IO_CACHE *info) { return MY_TEST(info->buffer); }
>  #define my_b_EOF INT_MIN
>  
> -#define my_b_read(info,Buffer,Count) \
> -  ((info)->read_pos + (Count) <= (info)->read_end ?\
> -   (memcpy(Buffer,(info)->read_pos,(size_t) (Count)), \
> -    ((info)->read_pos+=(Count)), 0) : _my_b_read((info), (Buffer), (Count)))
> -
> -#define my_b_write(info,Buffer,Count) \
> - ((info)->write_pos + (Count) <=(info)->write_end ?\
> -  (memcpy((info)->write_pos, (Buffer), (size_t)(Count)),\
> -   ((info)->write_pos+=(Count)), 0) : _my_b_write((info), (Buffer), (Count)))
> -
> -#define my_b_get(info) \
> -  ((info)->read_pos != (info)->read_end ?\
> -   ((info)->read_pos++, (int) (uchar) (info)->read_pos[-1]) :\
> -   _my_b_get(info))
> -
> -	/* my_b_write_byte dosn't have any err-check */
> -#define my_b_write_byte(info,chr) \
> -  (((info)->write_pos < (info)->write_end) ?\
> -   ((*(info)->write_pos++)=(chr)) :\
> -   ((my_b_flush_io_cache(info, 1)), ((*(info)->write_pos++)=(chr))))
> -
> -#define my_b_tell(info) ((info)->pos_in_file + \
> -			 (size_t) (*(info)->current_pos - (info)->request_pos))
> -#define my_b_write_tell(info) ((info)->pos_in_file + \
> -			 ((info)->write_pos - (info)->write_buffer))
> -
> -#define my_b_get_buffer_start(info) (info)->request_pos 
> -#define my_b_get_bytes_in_buffer(info) (char*) (info)->read_end -   \
> -  (char*) my_b_get_buffer_start(info)
> -#define my_b_get_pos_in_file(info) (info)->pos_in_file
> -
> -/* tell write offset in the SEQ_APPEND cache */
> +static inline int my_b_read(IO_CACHE *info, uchar *Buffer, size_t Count)
> +{
> +  if (info->read_pos + Count <= info->read_end)
> +  {
> +    memcpy(Buffer, info->read_pos, Count);
> +    info->read_pos+= Count;
> +    return 0;
> +  }
> +  return _my_b_read(info, Buffer, Count);
> +}
> +
> +static inline int my_b_write(IO_CACHE *info, const uchar *Buffer, size_t Count)
> +{
> +  if (info->write_pos + Count <= info->write_end)
> +  {
> +    memcpy(info->write_pos, Buffer, Count);
> +    info->write_pos+= Count;
> +    return 0;
> +  }
> +  return _my_b_write(info, Buffer, Count);
> +}
> +
> +static inline int my_b_get(IO_CACHE *info)
> +{
> +  if (info->read_pos != info->read_end)
> +  {
> +    info->read_pos++;
> +    return info->read_pos[-1];
> +  }
> +  return _my_b_get(info);
> +}
> +
> +/* my_b_write_byte dosn't have any err-check */
> +static inline void my_b_write_byte(IO_CACHE *info, uchar chr)
> +{
> +  if (info->write_pos >= info->write_end)
> +    my_b_flush_io_cache(info, 1);
> +  *info->write_pos++= chr;
> +}
> +
> +/**
> +  Fill buffer of the cache.
> +
> +  @note It assumes that you have already used all characters in the CACHE,
> +        independent of the read_pos value!
> +
> +  @returns
> +        0     On error or EOF (info->error = -1 on error)
> +        #     Number of characters
> +*/
> +static inline size_t my_b_fill(IO_CACHE *info)
> +{
> +  info->read_pos= info->read_end;
> +  return _my_b_read(info,0,0) ? 0 : info->read_end - info->read_pos;
> +}
> +
> +static inline my_off_t my_b_tell(const IO_CACHE *info)
> +{
> +  return info->pos_in_file + (*info->current_pos - info->request_pos);
> +}
> +
> +static inline my_off_t my_b_write_tell(const IO_CACHE *info)
> +{
> +  return info->pos_in_file + (info->write_pos - info->write_buffer);
> +}
> +
> +static inline uchar* my_b_get_buffer_start(const IO_CACHE *info)
> +{
> +  return info->request_pos;
> +}
> +
> +static inline size_t my_b_get_bytes_in_buffer(const IO_CACHE *info)
> +{
> +  return info->read_end - info->request_pos;
> +}
> +
> +static inline my_off_t my_b_get_pos_in_file(const IO_CACHE *info)
> +{
> +  return info->pos_in_file;
> +}
> +
> +static inline size_t my_b_bytes_in_cache(const IO_CACHE *info)
> +{
> +  return *info->current_end - *info->current_pos;
> +}
> +
>  int      my_b_copy_to_file(IO_CACHE *cache, FILE *file);
>  my_off_t my_b_append_tell(IO_CACHE* info);
>  my_off_t my_b_safe_tell(IO_CACHE* info); /* picks the correct tell() */
>  
> -#define my_b_bytes_in_cache(info) (size_t) (*(info)->current_end - \
> -					  *(info)->current_pos)
> -
>  typedef uint32 ha_checksum;
>  extern ulong my_crc_dbug_check;
>  
> @@ -741,24 +799,19 @@ extern my_bool reinit_io_cache(IO_CACHE *info,enum cache_type type,
>  			       my_off_t seek_offset, my_bool use_async_io,
>  			       my_bool clear_cache);
>  extern void setup_io_cache(IO_CACHE* info);
> -extern int _my_b_read(IO_CACHE *info,uchar *Buffer,size_t Count);
>  extern void init_io_cache_share(IO_CACHE *read_cache, IO_CACHE_SHARE *cshare,
>                                  IO_CACHE *write_cache, uint num_threads);
>  extern void remove_io_thread(IO_CACHE *info);
> -extern int _my_b_get(IO_CACHE *info);
>  extern int _my_b_async_read(IO_CACHE *info,uchar *Buffer,size_t Count);
> -extern int _my_b_write(IO_CACHE *info,const uchar *Buffer,size_t Count);
>  extern int my_b_append(IO_CACHE *info,const uchar *Buffer,size_t Count);
>  extern int my_b_safe_write(IO_CACHE *info,const uchar *Buffer,size_t Count);
>  
>  extern int my_block_write(IO_CACHE *info, const uchar *Buffer,
>  			  size_t Count, my_off_t pos);
> -extern int my_b_flush_io_cache(IO_CACHE *info, int need_append_buffer_lock);
>  
>  #define flush_io_cache(info) my_b_flush_io_cache((info),1)
>  
>  extern int end_io_cache(IO_CACHE *info);
> -extern size_t my_b_fill(IO_CACHE *info);
>  extern void my_b_seek(IO_CACHE *info,my_off_t pos);
>  extern size_t my_b_gets(IO_CACHE *info, char *to, size_t max_length);
>  extern my_off_t my_b_filelength(IO_CACHE *info);
> diff --git a/include/thr_lock.h b/include/thr_lock.h
> index 2561709..bc916b8 100644
> --- a/include/thr_lock.h
> +++ b/include/thr_lock.h
> @@ -166,7 +166,6 @@ void thr_set_lock_wait_callback(void (*before_wait)(void),
>                                  void (*after_wait)(void));
>  
>  #ifdef WITH_WSREP
> -#include <my_sys.h>
>    typedef my_bool (* wsrep_thd_is_brute_force_fun)(void *, my_bool);
>    typedef int (* wsrep_abort_thd_fun)(void *, void *, my_bool);
>    typedef int (* wsrep_on_fun)(void *);
> diff --git a/mysys/mf_iocache2.c b/mysys/mf_iocache2.c
> index 06dfc9f..a642e11 100644
> --- a/mysys/mf_iocache2.c
> +++ b/mysys/mf_iocache2.c
> @@ -61,7 +61,6 @@ my_b_copy_to_file(IO_CACHE *cache, FILE *file)
>      if (my_fwrite(file, cache->read_pos, bytes_in_cache,
>                    MYF(MY_WME | MY_NABP)) == (size_t) -1)
>        DBUG_RETURN(1);
> -    cache->read_pos= cache->read_end;
>    } while ((bytes_in_cache= my_b_fill(cache)));
>    if(cache->error == -1)
>      DBUG_RETURN(1);
> @@ -183,60 +182,6 @@ void my_b_seek(IO_CACHE *info,my_off_t pos)
>  
>  
>  /*
> -  Fill buffer of the cache.
> -
> -  NOTES
> -    This assumes that you have already used all characters in the CACHE,
> -    independent of the read_pos value!
> -
> -  RETURN
> -  0  On error or EOF (info->error = -1 on error)
> -  #  Number of characters
> -*/
> -
> -
> -size_t my_b_fill(IO_CACHE *info)
> -{
> -  my_off_t pos_in_file=(info->pos_in_file+
> -			(size_t) (info->read_end - info->buffer));
> -  size_t diff_length, length, max_length;
> -
> -  if (info->seek_not_done)
> -  {					/* File touched, do seek */
> -    if (mysql_file_seek(info->file, pos_in_file, MY_SEEK_SET, MYF(0)) ==
> -	MY_FILEPOS_ERROR)
> -    {
> -      info->error= 0;
> -      return 0;
> -    }
> -    info->seek_not_done=0;
> -  }
> -  diff_length=(size_t) (pos_in_file & (IO_SIZE-1));
> -  max_length=(info->read_length-diff_length);
> -  if (max_length >= (info->end_of_file - pos_in_file))
> -    max_length= (size_t) (info->end_of_file - pos_in_file);
> -
> -  if (!max_length)
> -  {
> -    info->error= 0;
> -    return 0;					/* EOF */
> -  }
> -  DBUG_EXECUTE_IF ("simulate_my_b_fill_error",
> -                   {DBUG_SET("+d,simulate_file_read_error");});
> -  if ((length= mysql_file_read(info->file, info->buffer, max_length,
> -                       info->myflags)) == (size_t) -1)
> -  {
> -    info->error= -1;
> -    return 0;
> -  }
> -  info->read_pos=info->buffer;
> -  info->read_end=info->buffer+length;
> -  info->pos_in_file=pos_in_file;
> -  return length;
> -}
> -
> -
> -/*
>    Read a string ended by '\n' into a buffer of 'max_length' size.
>    Returns number of characters read, 0 on error.
>    last byte is set to '\0'
> diff --git a/sql/log.cc b/sql/log.cc
> index 826a4d1..5d71732 100644
> --- a/sql/log.cc
> +++ b/sql/log.cc
> @@ -6694,7 +6694,6 @@ int MYSQL_BIN_LOG::write_cache(THD *thd, IO_CACHE *cache)
>          return ER_ERROR_ON_WRITE;
>      status_var_add(thd->status_var.binlog_bytes_written, length);
>  
> -    cache->read_pos=cache->read_end;		// Mark buffer used up
>    } while ((length= my_b_fill(cache)));
>  
>    DBUG_ASSERT(carry == 0);
> diff --git a/sql/wsrep_binlog.cc b/sql/wsrep_binlog.cc
> index b71da70..3788a6c 100644
> --- a/sql/wsrep_binlog.cc
> +++ b/sql/wsrep_binlog.cc
> @@ -71,7 +71,6 @@ int wsrep_write_cache_buf(IO_CACHE *cache, uchar **buf, size_t *buf_len)
>  
>        memcpy(*buf + *buf_len, cache->read_pos, length);
>        *buf_len = total_length;
> -      cache->read_pos = cache->read_end;
>    } while ((cache->file >= 0) && (length = my_b_fill(cache)));
>  
>    if (reinit_io_cache(cache, WRITE_CACHE, saved_pos, 0, 0))
> @@ -200,7 +199,6 @@ static int wsrep_write_cache_once(wsrep_t*  const wsrep,
>  
>          memcpy(buf + used, cache->read_pos, length);
>          used = total_length;
> -        cache->read_pos = cache->read_end;
>      } while ((cache->file >= 0) && (length = my_b_fill(cache)));
>  
>      if (used > 0)
> @@ -270,7 +268,6 @@ static int wsrep_write_cache_inc(wsrep_t*  const wsrep,
>                                                cache->read_pos, length)))
>                  goto cleanup;
>  
> -        cache->read_pos = cache->read_end;
>      } while ((cache->file >= 0) && (length = my_b_fill(cache)));
>  
>      if (WSREP_OK == err) *len = total_length;
> @@ -397,7 +394,6 @@ void wsrep_dump_rbr_direct(THD* thd, IO_CACHE* cache)
>        WSREP_ERROR("Failed to write file '%s'", filename);
>        goto cleanup;
>      }
> -    cache->read_pos= cache->read_end;
>    } while ((cache->file >= 0) && (bytes_in_cache= my_b_fill(cache)));
>    if(cache->error == -1)
>    {
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits


Follow ups