Subject: Re: [PATCH v6 03/19] fsck: Provide a function to parse fsck message IDs

From: Junio C Hamano <[email-masked]>
Date: Fri, 19 Jun 2015 12:13:03 -0700

Johannes Schindelin <[email-masked]> writes:

> +#define MSG_ID(id, msg_type) { STR(id), FSCK_##msg_type },
>  static struct {
> +	const char *id_string;
>  	int msg_type;
>  } msg_id_info[FSCK_MSG_MAX + 1] = {
>  	FOREACH_MSG_ID(MSG_ID)
> -	{ -1 }
> +	{ NULL, -1 }
>  };
>  #undef MSG_ID
>  
> +static int parse_msg_id(const char *text, int len)
> +{
> +	int i, j;
> +
> +	if (len < 0)
> +		len = strlen(text);
> +
> +	for (i = 0; i < FSCK_MSG_MAX; i++) {

I wonder an array without sentinel at the end with ARRAY_SIZE() may
be a leaner way to do these, especially as this is all limited to
this single file.

> +		const char *key = msg_id_info[i].id_string;
> +		/* match id_string case-insensitively, without underscores. */
> +		for (j = 0; j < len; j++) {
> +			char c = *(key++);
> +			if (c == '_')
> +				c = *(key++);

s/if/while/ perhaps?

> +			if (toupper(text[j]) != c)

I know the performance would not matter very much but calling
toupper() for each letter in the user input FSCK_MSG_MAX times
sounds rather inefficient.

Would it make sense to make the caller upcase instead (or upcase
upfront in the function)?

> +				break;
> +		}
> +		if (j == len && !*key)
> +			return i;
> +	}
> +
> +	return -1;
> +}
> +
>  static int fsck_msg_type(enum fsck_msg_id msg_id,
>  	struct fsck_options *options)
>  {