Re: [Tarantool-patches] [PATCH 09/17] recovery: recovery_new -- don't throw exception

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

See 2 comments below.

> diff --git a/src/box/recovery.cc b/src/box/recovery.cc
> index 24036b7c1..1c7665f87 100644
> --- a/src/box/recovery.cc
> +++ b/src/box/recovery.cc
> @@ -77,24 +77,21 @@
>  /* {{{ Initial recovery */
>  
>  /**
> - * Throws an exception in  case of error.
> + * Returns NULL in case of error.
>   */
>  struct recovery *
>  recovery_new(const char *wal_dirname, bool force_recovery,
>  	     const struct vclock *vclock)
>  {
> -	struct recovery *r = (struct recovery *)
> -			calloc(1, sizeof(*r));
> +	struct recovery *r;
>  
> +	r = (struct recovery *)calloc(1, sizeof(*r));

1. Would look better, if you assign 'r' on the same line
where you declare it. It is not really necessary to declare
all variables in the beginning and assign them separately.
Looks ugly, IMO. At least this particular place.

> diff --git a/src/box/relay.cc b/src/box/relay.cc
> index 2e17d0476..a5bcd9580 100644
> --- a/src/box/relay.cc
> +++ b/src/box/relay.cc
> @@ -363,8 +363,10 @@ relay_final_join(int fd, uint64_t sync, struct vclock *start_vclock,
>  		relay_delete(relay);
>  	});
>  
> -	relay->r = recovery_new(cfg_gets("wal_dir"), false,
> -			       start_vclock);
> +	relay->r = recovery_new(cfg_gets("wal_dir"), false, start_vclock);
> +	if (relay->r == NULL)
> +		diag_raise();
> +
>  	vclock_copy(&relay->stop_vclock, stop_vclock);
>  
>  	int rc = cord_costart(&relay->cord, "final_join",
> @@ -717,8 +719,10 @@ relay_subscribe(struct replica *replica, int fd, uint64_t sync,
>  	});
>  
>  	vclock_copy(&relay->local_vclock_at_subscribe, &replicaset.vclock);
> -	relay->r = recovery_new(cfg_gets("wal_dir"), false,
> -			        replica_clock);

2. This may look like a nit, but you could keep these 2 lines unchanged.
The same about the hunk above. My arguments are the same - patch size and
git blame purity.

> +	relay->r = recovery_new(cfg_gets("wal_dir"), false, replica_clock);
> +	if (relay->r == NULL)
> +		diag_raise();
> +
>  	vclock_copy(&relay->tx.vclock, replica_clock);
>  	relay->version_id = replica_version_id;
>  
>