Hi Junio,
On 2015-06-22 19:37, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schinde...@gmx.de> writes:
>
>> diff --git a/fsck.c b/fsck.c
>> index 1a3f7ce..e81a342 100644
>> --- a/fsck.c
>> +++ b/fsck.c
>> @@ -64,30 +64,29 @@ enum fsck_msg_id {
>> #undef MSG_ID
>>
>> #define STR(x) #x
>> -#define MSG_ID(id, msg_type) { STR(id), FSCK_##msg_type },
>> +#define MSG_ID(id, msg_type) { STR(id), NULL, FSCK_##msg_type },
>> static struct {
>> const char *id_string;
>> + const char *lowercased;
>> int msg_type;
>> } msg_id_info[FSCK_MSG_MAX + 1] = {
>> FOREACH_MSG_ID(MSG_ID)
>> - { NULL, -1 }
>> + { NULL, NULL, -1 }
>> };
>> #undef MSG_ID
>>
>> static int parse_msg_id(const char *text)
>> {
>> - static char **lowercased;
>> int i;
>>
>> - if (!lowercased) {
>> + if (!msg_id_info[0].lowercased) {
>> /* convert id_string to lower case, without underscores. */
>> - lowercased = xmalloc(FSCK_MSG_MAX * sizeof(*lowercased));
>> for (i = 0; i < FSCK_MSG_MAX; i++) {
>> const char *p = msg_id_info[i].id_string;
>> int len = strlen(p);
>> char *q = xmalloc(len);
>>
>> - lowercased[i] = q;
>> + msg_id_info[i].lowercased = q;
>> while (*p)
>> if (*p == '_')
>> p++;
>> @@ -98,7 +97,7 @@ static int parse_msg_id(const char *text)
>> }
>>
>> for (i = 0; i < FSCK_MSG_MAX; i++)
>> - if (!strcmp(text, lowercased[i]))
>> + if (!strcmp(text, msg_id_info[i].lowercased))
>> return i;
>>
>> return -1;
>
> Heh, this was the first thing that came to my mind when I saw 03/19
> that lazily prepares downcased version (which is good) but do so in
> a separately allocated buffer (which is improved by this change) ;-)
>
> IOW, I think all of the above should have been part of 03/19, not
> "oops I belatedly realized that this way is better" fixup here.
Gaaaah. Wrong commit fixed up. Sorry. Will be fixed in v8.
>> +void fsck_set_msg_types(struct fsck_options *options, const char *values)
>> +{
>> + char *buf = xstrdup(values), *to_free = buf;
>> + int done = 0;
>> +
>> + while (!done) {
>> + int len = strcspn(buf, " ,|"), equal;
>> +
>> + done = !buf[len];
>> + if (!len) {
>> + buf++;
>> + continue;
>> + }
>> + buf[len] = '\0';
>> +
>> + for (equal = 0; equal < len &&
>> + buf[equal] != '=' && buf[equal] != ':'; equal++)
>
> Style. I'd format this more like so:
>
> for (equal = 0;
> equal < len && buf[equal] != '=' && buf[equal] != ':';
> equal++)
Will be fixed.
>> + buf[equal] = tolower(buf[equal]);
>> + buf[equal] = '\0';
>> +
>> + if (equal == len)
>> + die("Missing '=': '%s'", buf);
>> +
>> + fsck_set_msg_type(options, buf, buf + equal + 1);
>> + buf += len + 1;
>> + }
>> + free(to_free);
>> +}
>
> Overall, the change is good (and it was good in v6, too), and I
> think it has become simpler to follow the logic with the upfront
> downcasing.
Yep, I agree. I did not expect that, but it was worth the effort to compare the
two versions.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
