From a4c58d92bfee0cf2e35ab1c5e828a6d790108a69 Mon Sep 17 00:00:00 2001 From: Mark Lodato Date: Sat, 4 Sep 2010 11:09:57 -0400 Subject: use __attribute__ to catch printf format mistakes Use "__attribute__((format (printf,N,M)))", as is done in git, do catch mistakes in printf-style format strings. Signed-off-by: Mark Lodato --- cache.h | 1 + cgit.h | 1 + html.h | 3 +++ 3 files changed, 5 insertions(+) diff --git a/cache.h b/cache.h index ac9276b..5cfdb4f 100644 --- a/cache.h +++ b/cache.h @@ -30,6 +30,7 @@ extern int cache_process(int size, const char *path, const char *key, int ttl, extern int cache_ls(const char *path); /* Print a message to stdout */ +__attribute__((format (printf,1,2))) extern void cache_log(const char *format, ...); extern unsigned long hash_str(const char *str); diff --git a/cgit.h b/cgit.h index 4090cd4..8f84281 100644 --- a/cgit.h +++ b/cgit.h @@ -293,6 +293,7 @@ extern void cgit_diff_tree(const unsigned char *old_sha1, extern void cgit_diff_commit(struct commit *commit, filepair_fn fn); +__attribute__((format (printf,1,2))) extern char *fmt(const char *format,...); extern struct commitinfo *cgit_parse_commit(struct commit *commit); diff --git a/html.h b/html.h index 16d55ec..1135fb8 100644 --- a/html.h +++ b/html.h @@ -5,7 +5,10 @@ extern int htmlfd; extern void html_raw(const char *txt, size_t size); extern void html(const char *txt); + +__attribute__((format (printf,1,2))) extern void htmlf(const char *format,...); + extern void html_status(int code, const char *msg, int more_headers); extern void html_txt(const char *txt); extern void html_ntxt(int len, const char *txt); -- cgit 1.4.1 From e4ddc8f72b5a7d8c55a6c2042c7b7f945ba4b1a2 Mon Sep 17 00:00:00 2001 From: Mark Lodato Date: Sat, 4 Sep 2010 11:30:18 -0400 Subject: fix errors in printf-style format strings There were many places where the arguments to a printf-like function did not match the format string. Mostly, these were a missing 'l' flag, but there were three exceptions: - In ui-stats.c, a size_t argument must be printed. C99 has the "%zu" flag for this purpose, but not all compilers support this. Therefore, we mimic what git does - use a NO_C99_FORMAT Makefile variable. - In ui-stats.c, cgit_print_error() was called with a pointer instead of a character. - In ui-log.c, the "columns" argument was never used. Signed-off-by: Mark Lodato --- Makefile | 8 ++++++++ cgit.c | 2 +- ui-diff.c | 2 +- ui-log.c | 3 +-- ui-stats.c | 18 ++++++++++++------ ui-tree.c | 4 ++-- 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 2a15469..6c9d118 100644 --- a/Makefile +++ b/Makefile @@ -16,6 +16,11 @@ INSTALL = install # # Define NEEDS_LIBICONV if linking with libc is not enough (eg. Darwin). # +# Define NO_C99_FORMAT if your formatted IO functions (printf/scanf et.al.) +# do not support the 'size specifiers' introduced by C99, namely ll, hh, +# j, z, t. (representing long long int, char, intmax_t, size_t, ptrdiff_t). +# some C compilers supported these specifiers prior to C99 as an extension. +# #-include config.mak @@ -127,6 +132,9 @@ endif ifdef NO_STRCASESTR CFLAGS += -DNO_STRCASESTR endif +ifdef NO_C99_FORMAT + CFLAGS += -DNO_C99_FORMAT +endif ifdef NO_OPENSSL CFLAGS += -DNO_OPENSSL GIT_OPTIONS += NO_OPENSSL=1 diff --git a/cgit.c b/cgit.c index d6146e2..3b3f8d9 100644 --- a/cgit.c +++ b/cgit.c @@ -610,7 +610,7 @@ static void process_cached_repolist(const char *path) hash = hash_str(path); if (ctx.cfg.project_list) hash += hash_str(ctx.cfg.project_list); - cached_rc = xstrdup(fmt("%s/rc-%8x", ctx.cfg.cache_root, hash)); + cached_rc = xstrdup(fmt("%s/rc-%8lx", ctx.cfg.cache_root, hash)); if (stat(cached_rc, &st)) { /* Nothing is cached, we need to scan without forking. And diff --git a/ui-diff.c b/ui-diff.c index 0dcabe9..7ff7e46 100644 --- a/ui-diff.c +++ b/ui-diff.c @@ -92,7 +92,7 @@ static void print_fileinfo(struct fileinfo *info) info->old_path); html(""); if (info->binary) { - htmlf("bin%d -> %d bytes", + htmlf("bin%ld -> %ld bytes", info->old_size, info->new_size); return; } diff --git a/ui-log.c b/ui-log.c index 0536b23..41b5225 100644 --- a/ui-log.c +++ b/ui-log.c @@ -228,8 +228,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern commit->parents = NULL; } if (pager) { - htmlf("
", - columns); + html("
"); if (ofs > 0) { cgit_log_link("[prev]", NULL, NULL, ctx.qry.head, ctx.qry.sha1, ctx.qry.vpath, diff --git a/ui-stats.c b/ui-stats.c index 50c2540..946a6ea 100644 --- a/ui-stats.c +++ b/ui-stats.c @@ -5,6 +5,12 @@ #include "ui-shared.h" #include "ui-stats.h" +#ifdef NO_C99_FORMAT +#define SZ_FMT "%u" +#else +#define SZ_FMT "%zu" +#endif + #define MONTHS 6 struct authorstat { @@ -283,10 +289,10 @@ void print_combined_authorrow(struct string_list *authors, int from, int to, if (date) subtotal += (size_t)date->util; } - htmlf("%d", centerclass, subtotal); + htmlf("%ld", centerclass, subtotal); total += subtotal; } - htmlf("%d", rightclass, total); + htmlf("%ld", rightclass, total); } void print_authors(struct string_list *authors, int top, @@ -335,16 +341,16 @@ void print_authors(struct string_list *authors, int top, if (!date) html("0"); else { - htmlf("%d", date->util); + htmlf(""SZ_FMT"", (size_t)date->util); total += (size_t)date->util; } } - htmlf("%d", total); + htmlf("%ld", total); } if (top < authors->nr) print_combined_authorrow(authors, top, authors->nr - 1, - "Others (%d)", "left", "", "sum", period); + "Others (%ld)", "left", "", "sum", period); print_combined_authorrow(authors, 0, authors->nr - 1, "Total", "total", "sum", "sum", period); @@ -367,7 +373,7 @@ void cgit_show_stats(struct cgit_context *ctx) i = cgit_find_stats_period(code, &period); if (!i) { - cgit_print_error(fmt("Unknown statistics type: %c", code)); + cgit_print_error(fmt("Unknown statistics type: %c", code[0])); return; } if (i > ctx->repo->max_stats) { diff --git a/ui-tree.c b/ui-tree.c index 75ec9cb..0cdbf6d 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -67,7 +67,7 @@ static void print_binary_buffer(char *buf, unsigned long size) html("\n"); html(""); for (ofs = 0; ofs < size; ofs += ROWLEN, buf += ROWLEN) { - htmlf("
ofshex dumpascii
%04x", ofs); + htmlf("
%04lx", ofs); for (idx = 0; idx < ROWLEN && ofs + idx < size; idx++) htmlf("%*s%02x", idx == 16 ? 4 : 1, "", @@ -108,7 +108,7 @@ static void print_object(const unsigned char *sha1, char *path, const char *base html(")\n"); if (ctx.cfg.max_blob_size && size / 1024 > ctx.cfg.max_blob_size) { - htmlf("
blob size (%dKB) exceeds display size limit (%dKB).
", + htmlf("
blob size (%ldKB) exceeds display size limit (%dKB).
", size / 1024, ctx.cfg.max_blob_size); return; } -- cgit 1.4.1 From 25e8ba1996a7b5ea291c924b0990d706176f6fe6 Mon Sep 17 00:00:00 2001 From: Mark Lodato Date: Sat, 4 Sep 2010 11:49:30 -0400 Subject: ui-repolist: fix redefinition of _XOPEN_SOURCE Previously, ui-repolist.c set _GNU_SOURCE and then included a standard library before including . This was a problem, because redefined _XOPEN_SOURCE, which is set automatically by glibc when _GNU_SOURCE is set. However, already sets _GNU_SOURCE and includes both and , so there is no need to define _GNU_SOURCE or include either header within ui-repolist.c. Signed-off-by: Mark Lodato --- ui-repolist.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ui-repolist.c b/ui-repolist.c index 0a0b6ca..2c98668 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -6,12 +6,6 @@ * (see COPYING for full license text) */ -/* This is needed for strcasestr to be defined by */ -#define _GNU_SOURCE 1 -#include - -#include - #include "cgit.h" #include "html.h" #include "ui-shared.h" -- cgit 1.4.1 From d187b98557d91b874836f286b955ba76ab26fb02 Mon Sep 17 00:00:00 2001 From: Mark Lodato Date: Sat, 4 Sep 2010 14:18:16 -0400 Subject: prefer html_raw() to write() To make the code more consistent, and to not rely on the implementation of html(), always use html_raw(...) instead of write(htmlfd, ...). Signed-off-by: Mark Lodato --- html.c | 18 +++++++++--------- ui-blob.c | 4 ++-- ui-tree.c | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/html.c b/html.c index eaabf72..1305910 100644 --- a/html.c +++ b/html.c @@ -95,7 +95,7 @@ void html_txt(const char *txt) while(t && *t){ int c = *t; if (c=='<' || c=='>' || c=='&') { - write(htmlfd, txt, t - txt); + html_raw(txt, t - txt); if (c=='>') html(">"); else if (c=='<') @@ -116,7 +116,7 @@ void html_ntxt(int len, const char *txt) while(t && *t && len--){ int c = *t; if (c=='<' || c=='>' || c=='&') { - write(htmlfd, txt, t - txt); + html_raw(txt, t - txt); if (c=='>') html(">"); else if (c=='<') @@ -128,7 +128,7 @@ void html_ntxt(int len, const char *txt) t++; } if (t!=txt) - write(htmlfd, txt, t - txt); + html_raw(txt, t - txt); if (len<0) html("..."); } @@ -139,7 +139,7 @@ void html_attr(const char *txt) while(t && *t){ int c = *t; if (c=='<' || c=='>' || c=='\'' || c=='\"') { - write(htmlfd, txt, t - txt); + html_raw(txt, t - txt); if (c=='>') html(">"); else if (c=='<') @@ -163,8 +163,8 @@ void html_url_path(const char *txt) int c = *t; const char *e = url_escape_table[c]; if (e && c!='+' && c!='&' && c!='+') { - write(htmlfd, txt, t - txt); - write(htmlfd, e, 3); + html_raw(txt, t - txt); + html_raw(e, 3); txt = t+1; } t++; @@ -180,8 +180,8 @@ void html_url_arg(const char *txt) int c = *t; const char *e = url_escape_table[c]; if (e) { - write(htmlfd, txt, t - txt); - write(htmlfd, e, 3); + html_raw(txt, t - txt); + html_raw(e, 3); txt = t+1; } t++; @@ -249,7 +249,7 @@ int html_include(const char *filename) return -1; } while((len = fread(buf, 1, 4096, f)) > 0) - write(htmlfd, buf, len); + html_raw(buf, len); fclose(f); return 0; } diff --git a/ui-blob.c b/ui-blob.c index 667a451..ec435e1 100644 --- a/ui-blob.c +++ b/ui-blob.c @@ -52,7 +52,7 @@ int cgit_print_file(char *path, const char *head) if (!buf) return -1; buf[size] = '\0'; - write(htmlfd, buf, size); + html_raw(buf, size); return 0; } @@ -108,5 +108,5 @@ void cgit_print_blob(const char *hex, char *path, const char *head) } ctx.page.filename = path; cgit_print_http_headers(&ctx); - write(htmlfd, buf, size); + html_raw(buf, size); } diff --git a/ui-tree.c b/ui-tree.c index 0cdbf6d..0b1b531 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -46,7 +46,7 @@ static void print_text_buffer(const char *name, char *buf, unsigned long size) html("
");
 		ctx.repo->source_filter->argv[1] = xstrdup(name);
 		cgit_open_filter(ctx.repo->source_filter);
-		write(STDOUT_FILENO, buf, size);
+		html_raw(buf, size);
 		cgit_close_filter(ctx.repo->source_filter);
 		html("
\n"); return; -- cgit 1.4.1