Re: [PATCH 3/4] refs.c: move ref parsing code out of resolve_ref()
On Fri, Jul 25, 2014 at 11:12 PM, Ronnie Sahlberg wrote: >> diff --git a/refs.c b/refs.c >> index bec2bb1..2769f20 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -1533,6 +1533,105 @@ static int handle_missing_loose_ref(const char >> *refname, >> } >> } >> >> +int parse_ref(const char *path, struct strbuf *ref, >> + unsigned char *sha1, int *flag) > > Can you make this function static? > It is not used by anything outside of this series and thus making it > static avoids growing the public refs api. It's to be used by builtin/checkout.c in nd/multiple-work-trees. I could mark it static now and unmark it later, but I'd need to add the static prototype back in refs.c because in the next patch resolve_gitlink_ref() uses this function and resolve_gitlink_ref() is before parse_ref(). -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] refs.c: move ref parsing code out of resolve_ref()
Nice.
On Fri, Jul 25, 2014 at 3:43 AM, Nguyễn Thái Ngọc Duy wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> cache.h | 11
> refs.c | 204
> ++--
> 2 files changed, 120 insertions(+), 95 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 5ffbafb..40a63d9 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1003,6 +1003,17 @@ extern int read_ref(const char *refname, unsigned char
> *sha1);
> extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
> int reading, int *flag);
> extern char *resolve_refdup(const char *ref, unsigned char *sha1, int
> reading, int *flag);
> extern int resolve_ref(const char *refname, struct strbuf *result, unsigned
> char *sha1, int reading, int *flag);
> +/*
> + * Given a ref in "ref" and its path, returns
> + *
> + * -2 failed to open with ENOENT, the caller is responsible for
> + * checking missing loose ref (see resolve_ref for example)
> + * -1 if there's an error, "ref" can no longer be trusted, "flag" may
> + * be set. errno is valid.
> + * 0 this is a symref, "ref" now contains the linked ref
> + * +1 normal ref, "sha1" is valid
> + */
> +extern int parse_ref(const char *path, struct strbuf *ref, unsigned char
> *sha1, int *flag);
>
> extern int dwim_ref(const char *str, int len, unsigned char *sha1, char
> **ref);
> extern int dwim_log(const char *str, int len, unsigned char *sha1, char
> **ref);
> diff --git a/refs.c b/refs.c
> index bec2bb1..2769f20 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1533,6 +1533,105 @@ static int handle_missing_loose_ref(const char
> *refname,
> }
> }
>
> +int parse_ref(const char *path, struct strbuf *ref,
> + unsigned char *sha1, int *flag)
Can you make this function static?
It is not used by anything outside of this series and thus making it
static avoids growing the public refs api.
> +{
> + struct strbuf buffer = STRBUF_INIT;
> + struct stat st;
> + const char *buf;
> +
> + /*
> +* We might have to loop back here to avoid a race condition:
> +* first we lstat() the file, then we try to read it as a link
> +* or as a file. But if somebody changes the type of the file
> +* (file <-> directory <-> symlink) between the lstat() and
> +* reading, then we don't want to report that as an error but
> +* rather try again starting with the lstat().
> +*/
> +stat_ref:
> + if (lstat(path, &st) < 0)
> + return errno == ENOENT ? -2 : -1;
> +
> + /* Follow "normalized" - ie "refs/.." symlinks by hand */
> + if (S_ISLNK(st.st_mode)) {
> + struct strbuf new_path = STRBUF_INIT;
> + if (strbuf_readlink(&new_path, path, 256) < 0) {
> + strbuf_release(&new_path);
> + if (errno == ENOENT || errno == EINVAL)
> + /* inconsistent with lstat; retry */
> + goto stat_ref;
> + else
> + return -1;
> + }
> + if (starts_with(new_path.buf, "refs/") &&
> + !check_refname_format(new_path.buf, 0)) {
> + strbuf_reset(ref);
> + strbuf_addbuf(ref, &new_path);
> + if (flag)
> + *flag |= REF_ISSYMREF;
> + strbuf_release(&new_path);
> + return 0;
> + }
> + strbuf_release(&new_path);
> + }
> +
> + /* Is it a directory? */
> + if (S_ISDIR(st.st_mode)) {
> + errno = EISDIR;
> + return -1;
> + }
> +
> + /*
> +* Anything else, just open it and try to use it as
> +* a ref
> +*/
> + if (strbuf_read_file(&buffer, path, 256) < 0) {
> + strbuf_release(&buffer);
> + if (errno == ENOENT)
> + /* inconsistent with lstat; retry */
> + goto stat_ref;
> + else
> + return -1;
> + }
> + strbuf_rtrim(&buffer);
> +
> + /*
> +* Is it a symbolic ref?
> +*/
> + if (!skip_prefix(buffer.buf, "ref:", &buf)) {
> + int ret;
> + /*
> +* Please note that FETCH_HEAD has a second line
> +* containing other data.
> +*/
> + if (get_sha1_hex(buffer.buf, sha1) ||
> + (buffer.buf[40] != '\0' && !isspace(buffer.buf[40]))) {
> + if (flag)
> + *flag |= REF_ISBROKEN;
> + errno = EINVAL;
> + ret = -1;
> + } else
> + ret = 1;
> + strbuf_release(&buffer);
> + return ret;
> + }
> +
[PATCH 3/4] refs.c: move ref parsing code out of resolve_ref()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
cache.h | 11
refs.c | 204 ++--
2 files changed, 120 insertions(+), 95 deletions(-)
diff --git a/cache.h b/cache.h
index 5ffbafb..40a63d9 100644
--- a/cache.h
+++ b/cache.h
@@ -1003,6 +1003,17 @@ extern int read_ref(const char *refname, unsigned char
*sha1);
extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
int reading, int *flag);
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading,
int *flag);
extern int resolve_ref(const char *refname, struct strbuf *result, unsigned
char *sha1, int reading, int *flag);
+/*
+ * Given a ref in "ref" and its path, returns
+ *
+ * -2 failed to open with ENOENT, the caller is responsible for
+ * checking missing loose ref (see resolve_ref for example)
+ * -1 if there's an error, "ref" can no longer be trusted, "flag" may
+ * be set. errno is valid.
+ * 0 this is a symref, "ref" now contains the linked ref
+ * +1 normal ref, "sha1" is valid
+ */
+extern int parse_ref(const char *path, struct strbuf *ref, unsigned char
*sha1, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/refs.c b/refs.c
index bec2bb1..2769f20 100644
--- a/refs.c
+++ b/refs.c
@@ -1533,6 +1533,105 @@ static int handle_missing_loose_ref(const char *refname,
}
}
+int parse_ref(const char *path, struct strbuf *ref,
+ unsigned char *sha1, int *flag)
+{
+ struct strbuf buffer = STRBUF_INIT;
+ struct stat st;
+ const char *buf;
+
+ /*
+* We might have to loop back here to avoid a race condition:
+* first we lstat() the file, then we try to read it as a link
+* or as a file. But if somebody changes the type of the file
+* (file <-> directory <-> symlink) between the lstat() and
+* reading, then we don't want to report that as an error but
+* rather try again starting with the lstat().
+*/
+stat_ref:
+ if (lstat(path, &st) < 0)
+ return errno == ENOENT ? -2 : -1;
+
+ /* Follow "normalized" - ie "refs/.." symlinks by hand */
+ if (S_ISLNK(st.st_mode)) {
+ struct strbuf new_path = STRBUF_INIT;
+ if (strbuf_readlink(&new_path, path, 256) < 0) {
+ strbuf_release(&new_path);
+ if (errno == ENOENT || errno == EINVAL)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ if (starts_with(new_path.buf, "refs/") &&
+ !check_refname_format(new_path.buf, 0)) {
+ strbuf_reset(ref);
+ strbuf_addbuf(ref, &new_path);
+ if (flag)
+ *flag |= REF_ISSYMREF;
+ strbuf_release(&new_path);
+ return 0;
+ }
+ strbuf_release(&new_path);
+ }
+
+ /* Is it a directory? */
+ if (S_ISDIR(st.st_mode)) {
+ errno = EISDIR;
+ return -1;
+ }
+
+ /*
+* Anything else, just open it and try to use it as
+* a ref
+*/
+ if (strbuf_read_file(&buffer, path, 256) < 0) {
+ strbuf_release(&buffer);
+ if (errno == ENOENT)
+ /* inconsistent with lstat; retry */
+ goto stat_ref;
+ else
+ return -1;
+ }
+ strbuf_rtrim(&buffer);
+
+ /*
+* Is it a symbolic ref?
+*/
+ if (!skip_prefix(buffer.buf, "ref:", &buf)) {
+ int ret;
+ /*
+* Please note that FETCH_HEAD has a second line
+* containing other data.
+*/
+ if (get_sha1_hex(buffer.buf, sha1) ||
+ (buffer.buf[40] != '\0' && !isspace(buffer.buf[40]))) {
+ if (flag)
+ *flag |= REF_ISBROKEN;
+ errno = EINVAL;
+ ret = -1;
+ } else
+ ret = 1;
+ strbuf_release(&buffer);
+ return ret;
+ }
+ if (flag)
+ *flag |= REF_ISSYMREF;
+ while (isspace(*buf))
+ buf++;
+ if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
+ if (flag)
+ *flag |= REF_ISBROKEN;
+ strbuf_release(&buffer);
+ errno = EINVAL;
+ return -1;
+ }
+ strbuf_reset(ref);
+ strbuf_addstr(ref, buf);
+ strbuf_release(&buffer);
+ return 0;
+}
+
/*
