cgit_repobasename: convert to allocated result

cgit_repobasename has one user also in ui-shared.c.  Make it static
and remove the declaration from cgit.h.

Instead of the gnarly return pointer to now deallocated stack,
compute the valid part of the string using the incoming pointer,
then just allocate the right amount and copy it in.  Drop the
const on the return type now it's allocated.

Cover the fact the input may be garbage by returning NULL if so.

Comment the function at the start that the result may be NULL or
must be freed now.

Convert the only user, cgit_snapshot_prefix(), to the same return
convention and also comment him at the start that the result may
be NULL or must be freed.  Also change the return type to char *.

Convert his only users, get_ref_from_filename() and
cgit_print_snapshot()in ui-snapshot.c, to deal with the new
result convention.  cgit_print_snapshot() already did an
xstrdup() on him anyway, just remove it and check for NULL.

The reason triggering all this was

../ui-shared.c: In function ‘cgit_repobasename’:
../ui-shared.c:135:2: warning: ‘strncpy’ specified bound 1024 equals destination size [-Wstringop-truncation]
  strncpy(rvbuf, reponame, sizeof(rvbuf));
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ comment from John Keeping.

Signed-off-by: Andy Green <andy@warmcat.com>

diff --git a/cgit.h b/cgit.h
index 6feca688d91c3b09121bd4776720471bffbba76b..6e6750c579f6ee81829df853f3f71dcc3f21cad9 100644 (file)
--- a/cgit.h
+++ b/cgit.h
@@ -369,8 +369,6 @@ extern struct commitinfo *cgit_parse_commit(struct commit *commit);
 extern struct taginfo *cgit_parse_tag(struct tag *tag);
 extern void cgit_parse_url(const char *url);
 
-extern const char *cgit_repobasename(const char *reponame);
-
 extern int cgit_parse_snapshots_mask(const char *str);
 extern const struct object_id *cgit_snapshot_get_sig(const char *ref,
                                                     const struct cgit_snapshot_format *f);

diff --git a/ui-shared.c b/ui-shared.c
index a63dcb041d99cdf65d101bd98bcc19059e329d94..24fa9f7bd4691a496994310c46850f7a28ed73db 100644 (file)
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -127,35 +127,40 @@ char *cgit_pageurl(const char *reponame, const char *pagename,
        return cgit_fileurl(reponame, pagename, NULL, query);
 }
 
-const char *cgit_repobasename(const char *reponame)
+/* result is NULL or must be freed */
+static char *cgit_repobasename(const char *reponame)
 {
-       /* I assume we don't need to store more than one repo basename */
-       static char rvbuf[1024];
-       int p;
-       const char *rv;
-       strncpy(rvbuf, reponame, sizeof(rvbuf));
-       if (rvbuf[sizeof(rvbuf)-1])
-               die("cgit_repobasename: truncated repository name '%s'", reponame);
-       p = strlen(rvbuf)-1;
-       /* strip trailing slashes */
-       while (p && rvbuf[p] == '/') rvbuf[p--] = 0;
-       /* strip trailing .git */
-       if (p >= 3 && starts_with(&rvbuf[p-3], ".git")) {
-               p -= 3; rvbuf[p--] = 0;
-       }
-       /* strip more trailing slashes if any */
-       while ( p && rvbuf[p] == '/') rvbuf[p--] = 0;
-       /* find last slash in the remaining string */
-       rv = strrchr(rvbuf,'/');
-       if (rv)
-               return ++rv;
-       return rvbuf;
+       int last = strlen(reponame) - 1, n;
+       char *rv;
+
+       if (last < 1)
+               return NULL;
+
+       while (last && reponame[last] == '/')
+               last--;
+
+       if (last >= 3 && !strncmp(&reponame[last - 3], ".git", 3))
+               last -= 3;
+
+       while (last && reponame[last] == '/')
+               last--;
+
+       n = last;
+       while (n && reponame[n] != '/')
+               n--;
+
+       rv = xmalloc(last - n + 2);
+       strncpy(rv, &reponame[n], last - n + 1);
+       rv[last - n + 1] = '\0';
+
+       return rv;
 }
 
-const char *cgit_snapshot_prefix(const struct cgit_repo *repo)
+/* result is NULL or must be freed */
+char *cgit_snapshot_prefix(const struct cgit_repo *repo)
 {
        if (repo->snapshot_prefix)
-               return repo->snapshot_prefix;
+               return xstrdup(repo->snapshot_prefix);
 
        return cgit_repobasename(repo->url);
 }

diff --git a/ui-shared.h b/ui-shared.h
index 4d5978bb6f79b643ee296b69b061baf4955dc4cc..49c11fcd8bb0eea69b63609d1c433f48f1291f74 100644 (file)
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -78,7 +78,8 @@ extern void cgit_compose_snapshot_prefix(struct strbuf *filename,
                                         const char *base, const char *ref);
 extern void cgit_print_snapshot_links(const struct cgit_repo *repo,
                                      const char *ref, const char *separator);
-extern const char *cgit_snapshot_prefix(const struct cgit_repo *repo);
+/* result is NULL or must be freed */
+extern char *cgit_snapshot_prefix(const struct cgit_repo *repo);
 extern void cgit_add_hidden_formfields(int incl_head, int incl_search,
                                       const char *page);
 

diff --git a/ui-snapshot.c b/ui-snapshot.c
index 92c3277ad42d2410da2c89dca98eba2dff764e75..8b5f046bf4a27d679cbb1ca64cb688c0fe07d40e 100644 (file)
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -206,7 +206,7 @@ static const char *get_ref_from_filename(const struct cgit_repo *repo,
                                         const char *filename,
                                         const struct cgit_snapshot_format *format)
 {
-       const char *reponame;
+       char *reponame = NULL;
        struct object_id oid;
        struct strbuf snapshot = STRBUF_INIT;
        int result = 1;
@@ -215,9 +215,12 @@ static const char *get_ref_from_filename(const struct cgit_repo *repo,
        strbuf_setlen(&snapshot, snapshot.len - strlen(format->suffix));
 
        if (get_oid(snapshot.buf, &oid) == 0)
-               goto out;
+               goto out1;
 
        reponame = cgit_snapshot_prefix(repo);
+       if (!reponame)
+               goto out1;
+
        if (starts_with(snapshot.buf, reponame)) {
                const char *new_start = snapshot.buf;
                new_start += strlen(reponame);
@@ -241,6 +244,8 @@ static const char *get_ref_from_filename(const struct cgit_repo *repo,
        strbuf_release(&snapshot);
 
 out:
+       free(reponame);
+out1:
        return result ? strbuf_detach(&snapshot, NULL) : NULL;
 }
 
@@ -288,7 +293,15 @@ void cgit_print_snapshot(const char *head, const char *hex,
                hex = head;
 
        if (!prefix)
-               prefix = xstrdup(cgit_snapshot_prefix(ctx.repo));
+               prefix = cgit_snapshot_prefix(ctx.repo);
+
+       if (!prefix) {
+               cgit_print_error_page(500, "Internal Server Error",
+                               "Bad repo name");
+
+               goto out1;
+       }
+
 
        if (sig_filename)
                write_sig(f, hex, filename, sig_filename);
@@ -296,5 +309,7 @@ void cgit_print_snapshot(const char *head, const char *hex,
                make_snapshot(f, hex, prefix, filename);
 
        free(prefix);
+
+out1:
        free(adj_filename);
 }