Thread (21 messages) 21 messages, 3 authors, 2016-06-15

Re: [PATCH v6 6/7] git-reflog: add create and exists functions

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:05:35

On Mon, Jun 29, 2015 at 4:17 PM, David Turner [off-list ref] wrote:
quoted hunk ↗ jump to hunk
These are necessary because alternate ref backends might store reflogs
somewhere other than .git/logs.  Code that now directly manipulates
.git/logs should instead go through git-reflog.

In a moment, we will use these functions to make git stash work with
alternate ref backends.

Signed-off-by: David Turner <redacted>
---
diff --git a/builtin/reflog.c b/builtin/reflog.c
index c2eb8ff..3080865 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -13,6 +13,10 @@ static const char reflog_expire_usage[] =
+static int cmd_reflog_create(int argc, const char **argv, const char *prefix)
+{
+       int i, status = 0, start = 0;
+       struct strbuf err = STRBUF_INIT;
+
+       for (i = 1; i < argc; i++) {
+               const char *arg = argv[i];
+               if (!strcmp(arg, "--")) {
+                       i++;
+                       break;
+               }
+               else if (arg[0] == '-')
+                       usage(reflog_create_usage);
+               else
+                       break;
+       }
+
+       start = i;
+
+       if (argc - start < 1)
+               return error("Nothing to create?");
+
+       for (i = start; i < argc; i++) {
+               if (check_refname_format(argv[i], REFNAME_ALLOW_ONELEVEL))
+                       die("invalid ref format: %s", argv[i]);
+       }
+       for (i = start; i < argc; i++) {
+               if (safe_create_reflog(argv[i], &err, 1)) {
+                       error("could not create reflog %s: %s", argv[i],
+                             err.buf);
+                       status = 1;
+                       strbuf_release(&err);
This feels a bit dirty. While it's true that the current
implementation of strbuf_release() re-initializes the strbuf (so
you're not technically wrong by re-using it), that's an implementation
detail; and indeed, the strbuf_release() documentation says:

    Release a string buffer and the memory it used. You should not
    use the string buffer after using this function, unless you
    initialize it again.

Alternatives would be strbuf_reset() or declaring and releasing the
strbuf within the for-loop scope.
quoted hunk ↗ jump to hunk
+               }
+       }
+       return status;
+}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help