Re: [Tarantool-patches] [PATCH 1/3] wal: sanitize wal_mode

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

> diff --git a/src/box/wal.h b/src/box/wal.h
> index a8a16ba2e..470aa1cc8 100644
> --- a/src/box/wal.h
> +++ b/src/box/wal.h
> @@ -41,7 +41,29 @@ struct fiber;
>  struct wal_writer;
>  struct tt_uuid;
>  
> -enum wal_mode { WAL_NONE = 0, WAL_WRITE, WAL_FSYNC, WAL_MODE_MAX };
> +enum wal_mode {
> +	/**
> +	 * Do not write data at all.
> +	 */
> +	WAL_NONE,
> +
> +	/**
> +	 * Write without waiting the data to be
> +	 * flushed to the storage device.
> +	 */
> +	WAL_WRITE,
> +
> +	/**
> +	 * Write data and wait the record to be
> +	 * flushed to the storage device.
> +	 */
> +	WAL_FSYNC,
> +
> +	WAL_MODE_MAX
> +};
> +
> +/** String constants for the supported modes. */
> +extern const char *wal_mode_STRS[WAL_MODE_MAX];

1. Why do you need to declare in the header the array size?
The old version with [] was just fine. And less places to change
if becomes necessary.

2. Please, keep the global variable where it was. We usually group
global variable declarations. It was grouped with wal_dir_lock
below.

>  enum {
>  	/**
> @@ -52,9 +74,6 @@ enum {
>  	WAL_ROWS_PER_YIELD = 1 << 15,
>  };
>  
> -/** String constants for the supported modes. */
> -extern const char *wal_mode_STRS[];
> -
>  extern int wal_dir_lock;
>  
>  #if defined(__cplusplus)
>