* [PATCH] use strchrnul() in place of strchr() and strlen()
@ 2014-03-05 19:29 Rohit Mani
2014-03-06 20:54 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Rohit Mani @ 2014-03-05 19:29 UTC (permalink / raw)
To: git; +Cc: Rohit Mani
Avoid scanning strings twice, once with strchr() and then with
strlen(), by using strchrnul(). Update the conditional expressions
involving the return value of strchrnul() with a check for '\0'.
Signed-off-by: Rohit Mani <rohit.mani@outlook.com>
---
I plan to apply for the GSoC.
archive.c | 4 ++--
cache-tree.c | 16 +++++++---------
diff.c | 9 +++------
fast-import.c | 35 ++++++++++++++---------------------
match-trees.c | 11 ++++-------
parse-options.c | 5 +----
pretty.c | 5 ++---
remote-testsvn.c | 4 ++--
ws.c | 7 ++-----
9 files changed, 37 insertions(+), 59 deletions(-)
diff --git a/archive.c b/archive.c
index 346f3b2..d196215 100644
--- a/archive.c
+++ b/archive.c
@@ -259,8 +259,8 @@ static void parse_treeish_arg(const char **argv,
/* Remotes are only allowed to fetch actual refs */
if (remote) {
char *ref = NULL;
- const char *colon = strchr(name, ':');
- int refnamelen = colon ? colon - name : strlen(name);
+ const char *colon = strchrnul(name, ':');
+ int refnamelen = colon - name;
if (!dwim_ref(name, refnamelen, sha1, &ref))
die("no such ref: %.*s", refnamelen, name);
diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..21a13cf 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -121,11 +121,11 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
if (!it)
return;
- slash = strchr(path, '/');
+ slash = strchrnul(path, '/');
it->entry_count = -1;
- if (!slash) {
+ if (*slash == '\0') {
int pos;
- namelen = strlen(path);
+ namelen = slash - path;
pos = subtree_pos(it, path, namelen);
if (0 <= pos) {
cache_tree_free(&it->down[pos]->cache_tree);
@@ -554,9 +554,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
const char *slash;
struct cache_tree_sub *sub;
- slash = strchr(path, '/');
- if (!slash)
- slash = path + strlen(path);
+ slash = strchrnul(path, '/');
/* between path and slash is the name of the
* subtree to look for.
*/
@@ -564,10 +562,10 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
if (!sub)
return NULL;
it = sub->cache_tree;
- if (slash)
- while (*slash && *slash == '/')
+ if (*slash != '\0')
+ while (*slash != '\0' && *slash == '/')
slash++;
- if (!slash || !*slash)
+ if (*slash == '\0' || !*slash)
return it; /* prefix ended with slashes */
path = slash;
}
diff --git a/diff.c b/diff.c
index e800666..83a039b 100644
--- a/diff.c
+++ b/diff.c
@@ -3365,14 +3365,11 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va
if (c != '-')
return 0;
arg++;
- eq = strchr(arg, '=');
- if (eq)
- len = eq - arg;
- else
- len = strlen(arg);
+ eq = strchrnul(arg, '=');
+ len = eq - arg;
if (!len || strncmp(arg, arg_long, len))
return 0;
- if (eq) {
+ if (*eq != '\0') {
int n;
char *end;
if (!isdigit(*++eq))
diff --git a/fast-import.c b/fast-import.c
index 4fd18a3..449595d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1485,14 +1485,12 @@ static int tree_content_set(
unsigned int i, n;
struct tree_entry *e;
- slash1 = strchr(p, '/');
- if (slash1)
- n = slash1 - p;
- else
- n = strlen(p);
+ slash1 = strchrnul(p, '/');
+ n = slash1 - p;
+
if (!n)
die("Empty path component found in input");
- if (!slash1 && !S_ISDIR(mode) && subtree)
+ if (*slash1 == '\0' && !S_ISDIR(mode) && subtree)
die("Non-directories cannot have subtrees");
if (!root->tree)
@@ -1501,7 +1499,7 @@ static int tree_content_set(
for (i = 0; i < t->entry_count; i++) {
e = t->entries[i];
if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
- if (!slash1) {
+ if (*slash1 == '\0') {
if (!S_ISDIR(mode)
&& e->versions[1].mode == mode
&& !hashcmp(e->versions[1].sha1, sha1))
@@ -1552,7 +1550,7 @@ static int tree_content_set(
e->versions[0].mode = 0;
hashclr(e->versions[0].sha1);
t->entries[t->entry_count++] = e;
- if (slash1) {
+ if (*slash1 != '\0') {
e->tree = new_tree_content(8);
e->versions[1].mode = S_IFDIR;
tree_content_set(e, slash1 + 1, sha1, mode, subtree);
@@ -1576,11 +1574,8 @@ static int tree_content_remove(
unsigned int i, n;
struct tree_entry *e;
- slash1 = strchr(p, '/');
- if (slash1)
- n = slash1 - p;
- else
- n = strlen(p);
+ slash1 = strchrnul(p, '/');
+ n = slash1 - p;
if (!root->tree)
load_tree(root);
@@ -1594,7 +1589,7 @@ static int tree_content_remove(
for (i = 0; i < t->entry_count; i++) {
e = t->entries[i];
if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
- if (slash1 && !S_ISDIR(e->versions[1].mode))
+ if (*slash1 != '\0' && !S_ISDIR(e->versions[1].mode))
/*
* If p names a file in some subdirectory, and a
* file or symlink matching the name of the
@@ -1602,7 +1597,7 @@ static int tree_content_remove(
* exist and need not be deleted.
*/
return 1;
- if (!slash1 || !S_ISDIR(e->versions[1].mode))
+ if (*slash1 == '\0' || !S_ISDIR(e->versions[1].mode))
goto del_entry;
if (!e->tree)
load_tree(e);
@@ -1644,11 +1639,9 @@ static int tree_content_get(
unsigned int i, n;
struct tree_entry *e;
- slash1 = strchr(p, '/');
- if (slash1)
- n = slash1 - p;
- else
- n = strlen(p);
+ slash1 = strchrnul(p, '/');
+ n = slash1 - p;
+
if (!n && !allow_root)
die("Empty path component found in input");
@@ -1664,7 +1657,7 @@ static int tree_content_get(
for (i = 0; i < t->entry_count; i++) {
e = t->entries[i];
if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
- if (!slash1)
+ if (*slash1 == '\0')
goto found_entry;
if (!S_ISDIR(e->versions[1].mode))
return 0;
diff --git a/match-trees.c b/match-trees.c
index 7873cde..7cca0f6 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -182,13 +182,10 @@ static int splice_tree(const unsigned char *hash1,
enum object_type type;
int status;
- subpath = strchr(prefix, '/');
- if (!subpath)
- toplen = strlen(prefix);
- else {
- toplen = subpath - prefix;
+ subpath = strchrnul(prefix, '/');
+ toplen = subpath - prefix;
+ if (*subpath != '\0')
subpath++;
- }
buf = read_sha1_file(hash1, &type, &sz);
if (!buf)
@@ -215,7 +212,7 @@ static int splice_tree(const unsigned char *hash1,
if (!rewrite_here)
die("entry %.*s not found in tree %s",
toplen, prefix, sha1_to_hex(hash1));
- if (subpath) {
+ if (*subpath != '\0') {
status = splice_tree(rewrite_here, subpath, hash2, subtree);
if (status)
return status;
diff --git a/parse-options.c b/parse-options.c
index 7b8d3fa..a5fa0b8 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -223,13 +223,10 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
const struct option *options)
{
const struct option *all_opts = options;
- const char *arg_end = strchr(arg, '=');
+ const char *arg_end = strchrnul(arg, '=');
const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
int abbrev_flags = 0, ambiguous_flags = 0;
- if (!arg_end)
- arg_end = arg + strlen(arg);
-
for (; options->type != OPTION_END; options++) {
const char *rest, *long_name = options->long_name;
int flags = 0, opt_flags = 0;
diff --git a/pretty.c b/pretty.c
index 87db08b..b1f60f9 100644
--- a/pretty.c
+++ b/pretty.c
@@ -549,14 +549,13 @@ static char *get_header(const struct commit *commit, const char *msg,
const char *line = msg;
while (line) {
- const char *eol = strchr(line, '\n'), *next;
+ const char *eol = strchrnul(line, '\n'), *next;
if (line == eol)
return NULL;
- if (!eol) {
+ if (*eol == '\0') {
warning("malformed commit (header is missing newline): %s",
sha1_to_hex(commit->object.sha1));
- eol = line + strlen(line);
next = NULL;
} else
next = eol + 1;
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 078f1ff..6be55cb 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -78,8 +78,8 @@ static int parse_rev_note(const char *msg, struct rev_note *res)
size_t len;
while (*msg) {
- end = strchr(msg, '\n');
- len = end ? end - msg : strlen(msg);
+ end = strchrnul(msg, '\n');
+ len = end - msg;
key = "Revision-number: ";
if (starts_with(msg, key)) {
diff --git a/ws.c b/ws.c
index b498d75..ea4b2b1 100644
--- a/ws.c
+++ b/ws.c
@@ -33,11 +33,8 @@ unsigned parse_whitespace_rule(const char *string)
int negated = 0;
string = string + strspn(string, ", \t\n\r");
- ep = strchr(string, ',');
- if (!ep)
- len = strlen(string);
- else
- len = ep - string;
+ ep = strchrnul(string, ',');
+ len = ep - string;
if (*string == '-') {
negated = 1;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] use strchrnul() in place of strchr() and strlen()
2014-03-05 19:29 [PATCH] use strchrnul() in place of strchr() and strlen() Rohit Mani
@ 2014-03-06 20:54 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2014-03-06 20:54 UTC (permalink / raw)
To: Rohit Mani; +Cc: git
Rohit Mani <rohit.mani@outlook.com> writes:
> Avoid scanning strings twice, once with strchr() and then with
> strlen(), by using strchrnul(). Update the conditional expressions
> involving the return value of strchrnul() with a check for '\0'.
>
> Signed-off-by: Rohit Mani <rohit.mani@outlook.com>
> ---
Nicely done. I am not sure if you need to say the "update the
conditional...", which is a logical consequence of such a conversion
and goes without saying, though.
> cache-tree.c | 16 +++++++---------
This part may overlap with other topics in flight, but I expect the
conflict resolution would be trivial.
> diff --git a/cache-tree.c b/cache-tree.c
> index 0bbec43..21a13cf 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -121,11 +121,11 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
>
> if (!it)
> return;
> - slash = strchr(path, '/');
> + slash = strchrnul(path, '/');
> it->entry_count = -1;
> - if (!slash) {
> + if (*slash == '\0') {
Let's just say
if (!*slash)
instead; it is more idiomatic (I won't repeat this for other hunks).
> int pos;
> - namelen = strlen(path);
> + namelen = slash - path;
After this "if (!*slash)", we compute "namelen = slash-path".
Perhaps we can lose this assignment and the other one by hoisting it
up before "if (!*slash)"?
> @@ -564,10 +562,10 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
> + if (*slash == '\0' || !*slash)
Huh? "The byte pointed at by 'slash' is NUL, or it is NUL"???
Other than that, looks good to me.
Thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-03-06 20:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-05 19:29 [PATCH] use strchrnul() in place of strchr() and strlen() Rohit Mani
2014-03-06 20:54 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).