[PATCH v2 2/2] cgit_repobasename: convert to allocated result
Andy Green
andy at warmcat.com
Tue Jun 26 13:36:35 CEST 2018
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 at warmcat.com>
---
cgit.h | 2 --
ui-shared.c | 53 +++++++++++++++++++++++++++++------------------------
ui-shared.h | 3 ++-
ui-snapshot.c | 21 ++++++++++++++++++---
4 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/cgit.h b/cgit.h
index 6feca68..6e6750c 100644
--- 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 ba88106..54d428f 100644
--- 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 4d5978b..49c11fc 100644
--- 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 4f7c21a..c7c945d 100644
--- 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);
}
More information about the CGit
mailing list