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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 19, 2015, 19:13 UTC
Message-ID: <xmqq4mm38qbk.fsf@gitster.dls.corp.google.com>
In-Reply-To: <8129a20ee6befc74eb45b6225edaaa1b61647d08.1434720655.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 19 quoted lines

> +#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.

Show 6 quoted lines

> +		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)?

Show 12 quoted lines

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