On Mon, Feb 29, 2016 at 11:08:07PM -0300, Gabriel Souza Franco wrote:
> On Mon, Feb 29, 2016 at 5:30 AM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
> > Having said that, this *might* be a good opportunity to imitate the
> > skip_prefix() function. If there are enough similar code constructs, we
> > could simplify all of them by introducing the function
> >
> > skip_oid_hex(const char *str, struct object_id *oid, const char **out)
> >
> > that returns 1 if and only if an oid was parsed, and stores the pointer
> > after the oid in "out" (skipping an additional space if there is one)?
>
> I don't think there's any other place that accepts all of "<sha1>",
> "<sha1> <ref>" and "<ref>"
> based on a quick grep for get_oid_hex.
Yes, but there are places where we get_oid_hex and then skip past that,
which could use the skip_oid_hex function, like:
diff --git a/connect.c b/connect.c
index 0478631..ba22ee6 100644
--- a/connect.c
+++ b/connect.c
@@ -149,10 +149,9 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
continue;
}
- if (len < GIT_SHA1_HEXSZ + 2 || get_oid_hex(buffer, &old_oid) ||
- buffer[GIT_SHA1_HEXSZ] != ' ')
+ if (!skip_oid_hex(buffer, &old_oid, &name) ||
+ !skip_prefix(name, " ", &name))
die("protocol error: expected sha/ref, got '%s'", buffer);
- name = buffer + GIT_SHA1_HEXSZ + 1;
name_len = strlen(name);
if (len != name_len + GIT_SHA1_HEXSZ + 1) {
_But_, if you look at the context just below, we make another implicit
assumption about GIT_SHA1_HEXSZ. So it's really not buying us that much
(unless we switch around the whole function to keep reading to the final
pointer, and compare "end - start" to the original "len" here).
So I'm not sure it's worth the trouble. I am really happy with the
skip_prefix() function for parsing like this, but I think it's just not
nearly as big a deal with oid-parsing, because we already have a nice
constant of GIT_SHA1_HEXSZ to match it (whereas skipping "foo" requires
us writing the magical "3" somewhere).
Anyway. Whether we want to pursue that or not, I don't think it needs to
be part of your series. Let's focus on the original goal.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html