[Devel] General handling of special cases if perferred

Kir Kolyshkin kir at openvz.org
Sun Apr 28 21:43:07 PDT 2013


On 04/28/2013 08:31 PM, Igor Podlesny wrote:
> ---
>   src/lib/util.c |   19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/src/lib/util.c b/src/lib/util.c
> index 8cc066b..420a574 100644
> --- a/src/lib/util.c
> +++ b/src/lib/util.c
> @@ -46,18 +46,17 @@ static const char *unescapestr(char *const src)
>   	char *p1, *p2;
>   	int fl = 0;
>   
> -	if (src == NULL)
> -		return NULL;

I don't like this approach.

In current version, a reviewer don't have to read the whole function
for the case when src is NULL, and it's clear we don't do anything in 
that case.

In your version, he have to read it in it's entirety, and it's a bit 
less clear that we are
returning NULL for src == NULL case, plus it's an extra indentation level.

For the rest of the patches concerning unescapestr() -- do you really 
fix anything
or it's just refactoring? If it's a pure refactoring, there should be a 
good reason for it,
which I missed from your comments.

> -	
> -	for (p1 = p2 = src; *p2; ++p2) {
> -		if (*p2 == '\\' && !fl)	{
> -			fl = 1;
> -		} else {
> -			*p1++ = *p2;
> -			fl = 0;
> +	if (src != NULL) {
> +		for (p1 = p2 = src; *p2; ++p2) {
> +			if (*p2 == '\\' && !fl)	{
> +				fl = 1;
> +			} else {
> +				*p1++ = *p2;
> +				fl = 0;
> +			}
>   		}
> +		*p1 = 0;
>   	}
> -	*p1 = 0;
>   
>   	return src;
>   }




More information about the Devel mailing list