{"thread":{"id":"21620","subject":"[PATCH] Check the format of more printf-type functions","startedAt":"2009-11-14T21:33:13Z","lastAt":"2009-11-15T14:17:26Z","messageCount":3,"participants":["Tarmigan Casebolt","Miklos Vajna","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127562","messageId":"1258234393-5093-1-git-send-email-tarmigan+git@gmail.com","threadId":"21620","inReplyTo":null,"subject":"[PATCH] Check the format of more printf-type functions","fromName":"Tarmigan Casebolt","fromEmail":"tarmigan+git@gmail.com","sentAt":"2009-11-14T21:33:13Z","receivedAt":"2009-11-14T21:33:13Z","isPatch":true,"sender":{"key":"tarmigan+git@gmail.com","avatar":null},"body":"We already have these checks in many printf-type functions that have\nprototypes which are in header files. Add these same checks to some\nmore prototypes in header functions and to static functions in .c\nfiles.\n\ncc: Miklos Vajna \nSigned-off-by: Tarmigan Casebolt \n---\n\nJunio, please consider this for next. It will hopefully catch some bugs like\nthe Content-Length one in http-backend.c.\n\nOne extra warning is\n CC merge-recursive.o\nmerge-recursive.c: In function ‘write_tree_from_memory’:\nmerge-recursive.c:218: warning: field precision should have type ‘int’, but argument 5 has type ‘size_t’\n\nA fix that might work in practice (because pathnames won't be longer than \nan int?) is:\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -215,7 +215,9 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n for (i = 0; i < active_nr; i++) {\n struct cache_entry *ce = active_cache[i];\n if (ce_stage(ce))\n- output(o, 0, \"%d %.*s\", ce_stage(ce), ce_namelen(ce), ce->name);\n+ output(o, 0, \"%d %.*s\", ce_stage(ce), (int)ce_namelen(ce), ce->name);\n+ if (ce_namelen(ce) > INT_MAX)\n+ die(\"A filename was too long\");\n }\n return NULL;\n }\n\nbut I don't think that is likely to be an acceptable fix, so I'm leaving\nit for others to make a proper fix. Looks like Miklos touched that line\nlast, so perhaps he knows of a better fix.\n\n builtin-fsck.c | 2 ++\n builtin-update-index.c | 1 +\n builtin-upload-archive.c | 1 +\n cache.h | 2 ++\n color.h | 2 ++\n daemon.c | 2 ++\n fsck.h | 1 +\n imap-send.c | 6 ++++++\n merge-recursive.c | 1 +\n strbuf.h | 2 +-\n 10 files changed, 19 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-fsck.c b/builtin-fsck.c\nindex 2d88e45..0e5faae 100644\n--- a/builtin-fsck.c\n+++ b/builtin-fsck.c\n@@ -47,6 +47,7 @@ static void objreport(struct object *obj, const char *severity,\n \tfputs(\"\\n\", stderr);\n }\n \n+__attribute__((format (printf, 2, 3)))\n static int objerror(struct object *obj, const char *err, ...)\n {\n \tva_list params;\n@@ -57,6 +58,7 @@ static int objerror(struct object *obj, const char *err, ...)\n \treturn -1;\n }\n \n+__attribute__((format (printf, 3, 4)))\n static int fsck_error_func(struct object *obj, int type, const char *err, ...)\n {\n \tva_list params;\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex 92beaaf..a6b7f2d 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -27,6 +27,7 @@ static int mark_valid_only;\n #define MARK_VALID 1\n #define UNMARK_VALID 2\n \n+__attribute__((format (printf, 1, 2)))\n static void report(const char *fmt, ...)\n {\n \tva_list vp;\ndiff --git a/builtin-upload-archive.c b/builtin-upload-archive.c\nindex c4cd1e1..b2d1259 100644\n--- a/builtin-upload-archive.c\n+++ b/builtin-upload-archive.c\n@@ -67,6 +67,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)\n \treturn write_archive(sent_argc, sent_argv, prefix, 0);\n }\n \n+__attribute__((format (printf, 1, 2)))\n static void error_clnt(const char *fmt, ...)\n {\n \tchar buf[1024];\ndiff --git a/cache.h b/cache.h\nindex 7cec30e..9fdf122 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -964,7 +964,9 @@ extern void *alloc_object_node(void);\n extern void alloc_report(void);\n \n /* trace.c */\n+__attribute__((format (printf, 1, 2)))\n extern void trace_printf(const char *format, ...);\n+__attribute__((format (printf, 2, 3)))\n extern void trace_argv_printf(const char **argv, const char *format, ...);\n \n /* convert.c */\ndiff --git a/color.h b/color.h\nindex 18abeb7..7d8da6f 100644\n--- a/color.h\n+++ b/color.h\n@@ -29,7 +29,9 @@ int git_color_default_config(const char *var, const char *value, void *cb);\n int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);\n void color_parse(const char *value, const char *var, char *dst);\n void color_parse_mem(const char *value, int len, const char *var, char *dst);\n+__attribute__((format (printf, 3, 4)))\n int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);\n+__attribute__((format (printf, 3, 4)))\n int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);\n int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);\n \ndiff --git a/daemon.c b/daemon.c\nindex 1b5ada6..641ebe1 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -77,6 +77,7 @@ static void logreport(int priority, const char *err, va_list params)\n \t}\n }\n \n+__attribute__((format (printf, 1, 2)))\n static void logerror(const char *err, ...)\n {\n \tva_list params;\n@@ -85,6 +86,7 @@ static void logerror(const char *err, ...)\n \tva_end(params);\n }\n \n+__attribute__((format (printf, 1, 2)))\n static void loginfo(const char *err, ...)\n {\n \tva_list params;\ndiff --git a/fsck.h b/fsck.h\nindex 008456b..1e4f527 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -17,6 +17,7 @@ typedef int (*fsck_walk_func)(struct object *obj, int type, void *data);\n /* callback for fsck_object, type is FSCK_ERROR or FSCK_WARN */\n typedef int (*fsck_error)(struct object *obj, int type, const char *err, ...);\n \n+__attribute__((format (printf, 3, 4)))\n int fsck_error_function(struct object *obj, int type, const char *fmt, ...);\n \n /* descend in all linked child objects\ndiff --git a/imap-send.c b/imap-send.c\nindex 6c9938a..854b6a4 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -102,13 +102,16 @@ struct msg_data {\n \n static int Verbose, Quiet;\n \n+__attribute__((format (printf, 1, 2)))\n static void imap_info(const char *, ...);\n+__attribute__((format (printf, 1, 2)))\n static void imap_warn(const char *, ...);\n \n static char *next_arg(char **);\n \n static void free_generic_messages(struct message *);\n \n+__attribute__((format (printf, 3, 4)))\n static int nfsnprintf(char *buf, int blen, const char *fmt, ...);\n \n static int nfvasprintf(char **strp, const char *fmt, va_list ap)\n@@ -560,6 +563,7 @@ static struct imap_cmd *v_issue_imap_cmd(struct imap_store *ctx,\n \treturn cmd;\n }\n \n+__attribute__((format (printf, 3, 4)))\n static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx,\n \t\t\t\t struct imap_cmd_cb *cb,\n \t\t\t\t const char *fmt, ...)\n@@ -573,6 +577,7 @@ static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx,\n \treturn ret;\n }\n \n+__attribute__((format (printf, 3, 4)))\n static int imap_exec(struct imap_store *ctx, struct imap_cmd_cb *cb,\n \t\t const char *fmt, ...)\n {\n@@ -588,6 +593,7 @@ static int imap_exec(struct imap_store *ctx, struct imap_cmd_cb *cb,\n \treturn get_cmd_result(ctx, cmdp);\n }\n \n+__attribute__((format (printf, 3, 4)))\n static int imap_exec_m(struct imap_store *ctx, struct imap_cmd_cb *cb,\n \t\t const char *fmt, ...)\n {\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex f55b7eb..d198312 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -86,6 +86,7 @@ static void flush_output(struct merge_options *o)\n \t}\n }\n \n+__attribute__((format (printf, 3, 4)))\n static void output(struct merge_options *o, int v, const char *fmt, ...)\n {\n \tint len;\ndiff --git a/strbuf.h b/strbuf.h\nindex d05e056..fa07ecf 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -117,7 +117,7 @@ struct strbuf_expand_dict_entry {\n };\n extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context);\n \n-__attribute__((format(printf,2,3)))\n+__attribute__((format (printf,2,3)))\n extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n \n extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);\n-- \n1.6.5.52.g4544ce0\n"},{"id":"127576","messageId":"20091115011044.GL23718@genesis.frugalware.org","threadId":"21620","inReplyTo":"1258234393-5093-1-git-send-email-tarmigan+git@gmail.com","subject":"Re: [PATCH] Check the format of more printf-type functions","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-11-15T01:10:44Z","receivedAt":"2009-11-15T01:10:44Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sat, Nov 14, 2009 at 01:33:13PM -0800, Tarmigan Casebolt wrote:\n> it for others to make a proper fix. Looks like Miklos touched that line\n> last, so perhaps he knows of a better fix.\n\nI just added struct merge_options, the real change is commit a97e407\n(Keep rename/rename conflicts of intermediate merges while doing\nrecursive merge, 2007-03-31), adding Alex to Cc.\n"},{"id":"127613","messageId":"81b0412b0911150617x5ef81b1ao9236c49d549ef8ea@mail.gmail.com","threadId":"21620","inReplyTo":"1258234393-5093-1-git-send-email-tarmigan+git@gmail.com","subject":"Re: [PATCH] Check the format of more printf-type functions","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-11-15T14:17:26Z","receivedAt":"2009-11-15T14:17:26Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Sat, Nov 14, 2009 at 22:33, Tarmigan Casebolt wrote:\n> We already have these checks in many printf-type functions that have\n> prototypes which are in header files.  Add these same checks to some\n> more prototypes in header functions and to static functions in .c\n> files.\n>\n> cc: Miklos Vajna \n> Signed-off-by: Tarmigan Casebolt \n> ---\n>\n> Junio, please consider this for next.  It will hopefully catch some bugs like\n> the Content-Length one in http-backend.c.\n>\n> One extra warning is\n>    CC merge-recursive.o\n> merge-recursive.c: In function ‘write_tree_from_memory’:\n> merge-recursive.c:218: warning: field precision should have type ‘int’, but argument 5 has type ‘size_t’\n>\n> A fix that might work in practice (because pathnames won't be longer than\n> an int?) is:\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -215,7 +215,9 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n>                for (i = 0; i < active_nr; i++) {\n>                        struct cache_entry *ce = active_cache[i];\n>                        if (ce_stage(ce))\n> -                               output(o, 0, \"%d %.*s\", ce_stage(ce), ce_namelen(ce), ce->name);\n> +                               output(o, 0, \"%d %.*s\", ce_stage(ce), (int)ce_namelen(ce), ce->name);\n\nIt'll do. The message is purely diagnostics.\n\n> +                       if (ce_namelen(ce) > INT_MAX)\n> +                               die(\"A filename was too long\");\n\nThat's overdoing it a little.\n"}]}