ssh-keygen -R is case-sensitive, but should not be

[Ángel González]

On 16/04/16 00:32, Griff Miller II wrote:
> Here is a better patch. Somehow I pasted an older version of my edits:

Hello Griff

Thanks for your patch. That will surely help the maintainers.
Here are a few comments from my part:

> -------------------------------------------------------
> % diff ./match.c /home/millerig/osrc/openssh-7.2p2/match.c
> 121a122
I'd have prefered an unified diff :)


>> 	char *low_string = 0;
I know that the C standard guarantees that it has the exact
same semantic, but please, if you are initializing a pointer, use NULL.

(PS: there's exactly an instance of that in openssh code at 
server_input_hostkeys_prove, that should be changed, too)

There's a memory leak on line 147 when a subpattern is too long.


> 156,159c157,168
> <  		if (match_pattern(string, sub)) {
> <  			if (negated)
> <  				return -1;		/* Negative */
> <  			else
> ---
>> 		if (dolower) {
By performing the lowercase here, you are doing a free() + malloc() + 
tolower() copying of the string per subpattern.
Just move the lowercasing code before the for.

>> 			u_int j;
>> 			if (low_string) free(low_string);
Which then makes this innecessary.

>> 			low_string = malloc(strlen(string) + 1);
Use xmalloc() here

>> 			for (j = 0; j<  strlen(string); ++j) low_string[j] = tolower(string[j]);
I'd recommend breaking this in two lines.

The compiler will probably figure out that strlen() can be called only 
once, instead of creating O(n²) code,
but this is equivalent to «for (j = 0; string[j]; …»

>> 			low_string[j] = 0;
Similar to the above NULL issue: low_string is a char*, so -although 
equivalent- it's generally better to use '\0'.

>> 		}
>> 		if (match_pattern((dolower ? low_string : string), sub)) {
>> 			if (negated) {
>> 				got_positive = -1;		/* Negative */
>> 				break;
>> 			} else
> 165,166c174,175
> <  	* Return success if got a positive match.  If there was a negative
> <  	* match, we have already returned -1 and never get here.
> ---
>> 	* Return success if there was a positive match;
>> 	 * return -1 if there was a negative match.
> 167a177
>> 	if (low_string) free(low_string);
Useless if

Best regards