Hi Junio,
On 2015-06-19 21:13, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> 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.
The real reason is that I cannot prevent a trailing comma with the way I construct the array, so FSCK_MSG_MAX is a natural way to support compilers that do not allow something like
enum { A, B, C, };
>> + 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?
Actually, I want to prevent double underscores so that no ambiguity occurs between the camelCased version of, say, JUNIO_HAMANO and JUNIO__HAMANO.
I inserted an `assert()`;
>> + 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)?
As you said, performance plays a lesser role here than simplicity. The strings we receive here are passed in read-only, therefore I would have to `strdup()` them first and then make sure to `free()` them. That was too un-simple for my taste.
Having said that, if you disagree, I will introduce the complexity, though. Do you want me to do that?
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in