List:Commits« Previous MessageNext Message »
From:Marko Mäkelä Date:November 18 2010 7:56am
Subject:Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)
View as plain text  
On Thu, Nov 18, 2010 at 05:11:06AM -0000, kevin.lewis@stripped wrote:
>@@ -6496,8 +6496,7 @@ create_clustered_index_when_no_primary(
> /*****************************************************************//**
> Return a display name for the row format
> @return row format name */
>-
>-const char *get_row_format_name(
>+const char *get_row_type_name(
> /*============================*/
> enum row_type row_format)		/*!< in: Row Format */
> {

This is still in violation of the InnoDB coding style. Let me list the 
violations:

1. The linkage specifier (UNIV_INTERN, UNIV_INLINE, static) is missing.
2. The return type and the function name have to be on separate lines.
3. The * or & is part of the type name. No space before it.
4. The /*====*/ comment length must match the previous line.
5. The parameters must be indented with TABs: in the beginning of the 
line, between the type and the name, and between the name and the 
comment.

Here is a correct example:

/*****************************************************************//**
Return a display name for the row format
@return row format name */
UNIV_INTERN
const char*
get_row_type_name(
/*==============*/
	enum row_type	row_format)	/*!< in: Row Format */
{
	...
}

More problems in your patch:

>+
>+#define CREATE_OPTION_MESSAGE(_text_, _parm_) {	\
>+	push_warning_printf(	\
>+		thd, MYSQL_ERROR::WARN_LEVEL_WARN,	\
>+		ER_ILLEGAL_HA_CREATE_OPTION,	\
>+		_text_, _parm_);	\
>+}

If you have to put {} around your code in macros (it is not necessary in 
this case), then do it as do {...} while (0). Otherwise your macro can 
break when expanded in certain contexts.

Document the macro parameters too. Never use identifiers that start with 
an underscore. Align the trailing \ with TABs. Here is a better try:

/** Issue a custom warning message for illegal create option.
@param text	warning text
@param parm	parameters for the warning */
#define CREATE_OPTION_MESSAGE(text, parm)		\
	push_warning_printf(				\
		thd, MYSQL_ERROR::WARN_LEVEL_WARN,	\
		ER_ILLEGAL_HA_CREATE_OPTION,		\
		_text_, _parm_)

Best regards,

	Marko

PS: I would like to get rid of the /*=====*/ comment. Does anyone need it?
PPS: I find the 3-column TAB aligned layout of function parameters and 
comments a little cumbersome. Could we move the parameter comments to 
@param clauses in the function comments, like in the above macro 
example?
Thread
bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222) kevin.lewis18 Nov
  • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Vasil Dimov18 Nov
    • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Kevin Lewis18 Nov
    • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Kevin Lewis18 Nov
  • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Marko Mäkelä18 Nov
    • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Kevin Lewis18 Nov
      • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Calvin Sun18 Nov
      • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Marko Mäkelä18 Nov
        • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Sunny Bains18 Nov
          • RE: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Vladislav Vaintroub18 Nov
          • RE: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Vladislav Vaintroub18 Nov
          • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Sergei Golubchik18 Nov
          • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Marko Mäkelä22 Nov
          • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Marko Mäkelä22 Nov
        • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Sunny Bains18 Nov
      • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Marko Mäkelä18 Nov
      • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Calvin Sun18 Nov
    • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Kevin Lewis18 Nov
  • Re: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222)Marko Mäkelä18 Nov