[PATCH] filter: refactor cgit_new_filter()
From: Lukas Fleischer c...@cryptocrack.de
Use prefixcmp() as a preparation for using strip_prefix() later. Also,
interpret the command as a file name if it contains a colon but none of
the filter prefixes matches instead of bailing out and adding a special
check for Windows.
Signed-off-by: Lukas Fleischer c...@cryptocrack.de
---
John -- this is your code so I'm awaiting your input on this.
Personally, I like receiving an error message that says invalid filter
type, and this patch kills that. But I'll go with whatever you prefer.
filter.c | 30 +++---
1 file changed, 7 insertions(+), 23 deletions(-)
diff --git a/filter.c b/filter.c
index 4d4acaf..1997881 100644
--- a/filter.c
+++ b/filter.c
@@ -379,31 +379,19 @@ static const struct {
const char *prefix;
struct cgit_filter *(*ctor)(const char *cmd, int argument_count);
} filter_specs[] = {
- { exec, new_exec_filter },
+ { exec:, new_exec_filter },
#ifndef NO_LUA
- { lua, new_lua_filter },
+ { lua:, new_lua_filter },
#endif
};
struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
{
- char *colon;
- int i;
- size_t len;
- int argument_count;
+ int argument_count, i;
if (!cmd || !cmd[0])
return NULL;
- colon = strchr(cmd, ':');
- len = colon - cmd;
- /*
-* In case we're running on Windows, don't allow a single letter before
-* the colon.
-*/
- if (len == 1)
- colon = NULL;
-
switch (filtertype) {
case EMAIL:
argument_count = 2;
@@ -420,15 +408,11 @@ struct cgit_filter *cgit_new_filter(const char *cmd,
filter_type filtertype)
break;
}
- /* If no prefix is given, exec filter is the default. */
- if (!colon)
- return new_exec_filter(cmd, argument_count);
-
for (i = 0; i ARRAY_SIZE(filter_specs); i++) {
- if (len == strlen(filter_specs[i].prefix)
- !strncmp(filter_specs[i].prefix, cmd, len))
- return filter_specs[i].ctor(colon + 1, argument_count);
+ if (!prefixcmp(cmd, filter_specs[i].prefix))
+ return filter_specs[i].ctor(strchr(cmd, ':') + 1,
argument_count);
}
- die(Invalid filter type: %.*s, (int) len, cmd);
+ /* If no valid prefix is given, exec filter is the default. */
+ return new_exec_filter(cmd, argument_count);
}
--
1.8.5.2
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: refactor cgit_new_filter()
On Tue, Jan 14, 2014 at 02:00:48PM +0100, Jason A. Donenfeld wrote:
From: Lukas Fleischer c...@cryptocrack.de
Use prefixcmp() as a preparation for using strip_prefix() later. Also,
interpret the command as a file name if it contains a colon but none of
the filter prefixes matches instead of bailing out and adding a special
check for Windows.
Signed-off-by: Lukas Fleischer c...@cryptocrack.de
---
John -- this is your code so I'm awaiting your input on this.
Personally, I like receiving an error message that says invalid filter
type, and this patch kills that. But I'll go with whatever you prefer.
Yeah, it would be nice to keep the unrecognised prefix warning.
I like the simplification, but I'm not sure the result is better. Even
without the rest we should replace the strncmp with prefixcmp though.
There's actually no reason we couldn't mutate cmd here, which would
simplify it a lot, but I'm not sure we want to remove the const
modifiers all the way through. Then we can just do *colon = '\0' and
use strcmp.
filter.c | 30 +++---
1 file changed, 7 insertions(+), 23 deletions(-)
diff --git a/filter.c b/filter.c
index 4d4acaf..1997881 100644
--- a/filter.c
+++ b/filter.c
@@ -379,31 +379,19 @@ static const struct {
const char *prefix;
struct cgit_filter *(*ctor)(const char *cmd, int argument_count);
} filter_specs[] = {
- { exec, new_exec_filter },
+ { exec:, new_exec_filter },
#ifndef NO_LUA
- { lua, new_lua_filter },
+ { lua:, new_lua_filter },
#endif
};
struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
{
- char *colon;
- int i;
- size_t len;
- int argument_count;
+ int argument_count, i;
if (!cmd || !cmd[0])
return NULL;
- colon = strchr(cmd, ':');
- len = colon - cmd;
- /*
- * In case we're running on Windows, don't allow a single letter before
- * the colon.
- */
- if (len == 1)
- colon = NULL;
-
switch (filtertype) {
case EMAIL:
argument_count = 2;
@@ -420,15 +408,11 @@ struct cgit_filter *cgit_new_filter(const char *cmd,
filter_type filtertype)
break;
}
- /* If no prefix is given, exec filter is the default. */
- if (!colon)
- return new_exec_filter(cmd, argument_count);
-
for (i = 0; i ARRAY_SIZE(filter_specs); i++) {
- if (len == strlen(filter_specs[i].prefix)
- !strncmp(filter_specs[i].prefix, cmd, len))
- return filter_specs[i].ctor(colon + 1, argument_count);
+ if (!prefixcmp(cmd, filter_specs[i].prefix))
+ return filter_specs[i].ctor(strchr(cmd, ':') + 1,
argument_count);
Using strchr here feels wrong, why not:
cmd + strlen(filter_specs[i].prefix)
?
}
- die(Invalid filter type: %.*s, (int) len, cmd);
+ /* If no valid prefix is given, exec filter is the default. */
+ return new_exec_filter(cmd, argument_count);
}
--
1.8.5.2
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
___
CGit mailing list
CGit@lists.zx2c4.com
http://lists.zx2c4.com/mailman/listinfo/cgit
Re: [PATCH] filter: refactor cgit_new_filter()
On Tue, Jan 14, 2014 at 09:54:21PM +0100, Jason A. Donenfeld wrote: On Tue, Jan 14, 2014 at 9:39 PM, John Keeping j...@keeping.me.uk wrote: I like the simplification, but I'm not sure the result is better. Even without the rest we should replace the strncmp with prefixcmp though. Agreed. There's actually no reason we couldn't mutate cmd here, which would simplify it a lot, but I'm not sure we want to remove the const modifiers all the way through. Then we can just do *colon = '\0' and use strcmp. IMHO, it's better to keep lookup tables like these in the read-only section. Let's keep the constness. I meant for the input, which is read from the config file. Just terminate the user-specified command string at the colon and treat it as two genuinely separate strings. ___ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit
