Johannes Schindelin <johannes.schindelin@xxxxxx> 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.
The end result looks good, so let's keep reading.
> +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++)
> + 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.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in