List:Internals« Previous MessageNext Message »
From:Sergei Golubchik Date:July 24 2009 7:11am
Subject:Re: GSoC Week 11 - I_S/P_S storage engine
View as plain text  
Hi, scut_tang!

On Jul 24, scut_tang wrote:
> Hi, Sergei
> >you forgot to set share->blob_ptr_size, and because it was 0 here:
> >
> >  reclength += new_field->row_pack_length() + share->blob_ptr_size;
> >
> Fixed it.

better set it to portable_sizeof_char_ptr, not to sizeof(void*).

I suppose it makes no difference in our case, because the table is never
stored on disk, so there's no chance that your TABLE_SHARE will be saved
and later read on a system with a different sizeof(void*). So,
portability is not a concern.

But better use the same constant - portable_sizeof_char_ptr -
everywhere, to avoid questions about it (and somebody may copy-paste
your code and use sizeof(void*) where portability *is* a concern).
 
> >No, I can not. I said you many times - fix the formatting, especially
> >the indentation. After a few futile reminders I told you that I won't
> >review your code anymore until the indentation is fixed.
> >
> >Sorry, I cannot review your code now.
> I am so sorry! I think it is a misunderstanding.

> I used to program by the editor Emacs, but in this project I use GVIM
> and I am not familiar with GVIM.
> There is some problems in my GVIM configuration file(.vimrc). In my
> GVIM, all source code looks good and intents by 2 spaces (actually
> they are
> wrong), so I was puzzled why you always told me to format the source
> code. This morning, I try to use others editors, like Eclipse and
> Gedit to open my source code, and understand why I made you feel
> unhappy. I find all the source code is indented by 8 spaces in Eclipse
> and Gedit, so that it looks very bad, while in my GVIM, it turns out
> to be just 2 spaces. Finally, I add "set expandtabs" into .vimrc to
> use spaces instead of tabs, and it works well. The source code has
> been formatted by Artistic Style and new GVIM, and has been pushed to
> branch.
> It's my carelessness. So I beg your pardon.

no problem. you should've said earlier that you don't understand what
I'm talking about :)

I'm using vim (in fact, gvim) myself, below I'll show relevant pieces of
my .vimrc.

> You check it whether it looks normal?

few issues:

1.
ignoring all whitespace changes (too difficult to diff with my eyes :),
in ha_infoschema.cc you had

-        for(; fields_info->field_name; fields_info++)
-        {
+  for(; fields_info->field_name; fields_info++) {

the old one was correct, curly brace on a separate line.

2. table_character_sets.cc has dos line endings (cr-lf) not unix ones.
   (in vim: open a file, ":set ff=unix" save)

Now about vim. Below are a few lines from my .vimrc

====[$HOME/.vimrc]======================================================
set nocompatible
set autoindent          " always set autoindenting on
set expandtab           " always expand tabs
set shiftwidth=2

" Load all ../../../vimrc's
let &runtimepath=&runtimepath . 
",../../../../.vim,../../../.vim,../../.vim,../.vim,./.vim"
runtime! vimrc
========================================================================

note the last two lines, it's a trick that allows to have per-project
settings. E.g. if you have a directory, say, ~/GSoC and in there, say
~/GSoC/Infoschema you can create ~/GSoC/.vim directory, put your vimrc
(without a dot) there and this vimrc will only work for files inside
~/GSoC - this is how I have separate settings for MySQL source code and
for other projects. Of course you can also put your settings in ~/.vimrc
to have them everywhere. Anyway, here:

=======[$HOME/MySQL/.vim/vimrc]========================================
au! BufNewFile,BufRead *.test     set filetype=sql
au! BufNewFile,BufRead *.inc      set filetype=sql
au! BufNewFile,BufRead *.yy       set filetype=yacc
au! BufNewFile,BufRead *.ic       set filetype=c
au! BufNewFile,BufRead *.i        set filetype=c
au! BufNewFile,BufRead errmsg.txt set filetype=errmsg
au! BufNewFile,BufRead *.m4       set filetype=config

let yacc_uses_cpp=1

let bzrroot=system('echo -n `bzr root`')
let &tags= bzrroot . "/tags"
let &path=".," . bzrroot . "/include,/usr/include,," 
 
menu 90.500 B&ZR.&Revert        :!bzr revert --no-backup
%<CR><CR><CR>:e!<CR>
menu 90.600 B&ZR.&Diff          :!bzr gdiff % &<CR><CR>
menu 90.700 B&ZR.&Annotate      :!bzr gannotate % &<CR><CR>
menu 90.800 B&ZR.&Revgraph      :!bzr revgraph % &<CR><CR>
========================================================================

this sets up file extensions, create a BZR menu in gvim, and sets up
a path for include files.

=======[$HOME/MySQL/.vim/ftplugin/c.vim]===============================
setlocal cino=:0,c2,(0      " C indenting style
setlocal comments=://       " Comments that auto-formatting should respect
========================================================================

this one ^^^^ configures the formatting

=======[$HOME/MySQL/.vim/syntax/asm.vim]===============================
syn match asmComment            "#.*"
=======================================================================

=======[$HOME/MySQL/.vim/syntax/c.vim]=================================
syn keyword     cType   uint uchar my_off_t byte my_bool ulong ulonglong longlong
syn keyword     cType   int8 int16 int32 int64 uint8 uint16 uint32 uint64 intptr

syn keyword     cStruct TREE TREE_ELEMENT HASH QUEUE LIST MEM_ROOT
syn keyword     cStruct MI_INFO MI_KEYDEF FTB_WORD FTB_EXPR FTB
syn keyword     cStruct FT_WORD FT_INFO FTB_PARAM FT_SEG_ITERATOR
syn keyword     cStruct FT_DOC FT_SUPERDOC

syn keyword     cConstant HA_FT_WLEN MI_MAX_KEY_BUFF HA_ERR_END_OF_FILE
syn keyword     cConstant MY_WME MY_ZEROFILL MY_FAE MY_WAIT_IF_FULL MY_FNABP MY_NABP
syn keyword     cConstant HA_POS_ERROR TRUE FALSE EDQUOT

syn keyword     cTodo QQ SergeiTODO

syn keyword     cSpecial DBUG_ENTER DBUG_RETURN DBUG_ASSERT DBUG_PRINT DBUG_VOID_RETURN

hi def link cStruct     Macro
=======================================================================

Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring
Thread
Re: GSoC Week 11 - I_S/P_S storage engineSergei Golubchik24 Jul