Re: [PATCH/RFC 1/2] is_url: Remove redundant assignment

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 26, 2011, 16:52 UTC
Message-ID: <7vsjnj455l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALUzUxp-++A7azzimE1mjvPVoGxRM4rzYt9z66CmFb0B3vi7dQ@mail.gmail.com>

Tay Ray Chuan <rctay89@gmail.com> writes:

Show 23 quoted lines

> On Sun, Sep 25, 2011 at 1:06 PM, Pang Yan Han <pangyanhan@gmail.com> wrote:
>> Signed-off-by: Pang Yan Han <pangyanhan@gmail.com>
>> ---
>>  url.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/url.c b/url.c
>> index 3e06fd3..d2e17e6 100644
>> --- a/url.c
>> +++ b/url.c
>> @@ -22,7 +22,6 @@ int is_url(const char *url)
>>
>>        if (!url)
>>                return 0;
>> -       url2 = url;
>>        first_slash = strchr(url, '/');
>>
>>        /* Input with no slash at all or slash first can't be URL. */
>
> Looks correct. Perhaps you could mention in the patch message that
>
>   There are no operations on url2 until another assignment to it later
> at line 41.

The looks correct, so I'll queue it, but it looks like that the function is implemented in an overly complicated way.

Why aren't we checking from left to right in a single pass, perhaps like this?

	/* Make sure it is of form "scheme://something" */
	int is_url(const char *url)
	{
		/* Is "scheme" part reasonable? */
		if (!url || !is_urlschemechar(1, *url++))
	        	return 0;
		while (*url && *url != ':') {
			if (!is_urlschemechar(0, *url++))
				return 0;
		}
		/* We've seen "scheme"; we want colon-slash-slash */
		return (url[0] == ':' && url[1] == '/' && url[2] == '/');
	}