{"thread":{"id":"34476","subject":"[PATCH] Add the GIT_SENTINEL macro","startedAt":"2013-07-18T20:02:12Z","lastAt":"2013-07-20T19:13:10Z","messageCount":7,"participants":["Ramsay Jones","Jeff King","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"223693","messageId":"51E849C4.7020305@ramsay1.demon.co.uk","threadId":"34476","inReplyTo":null,"subject":"[PATCH] Add the GIT_SENTINEL macro","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2013-07-18T20:02:12Z","receivedAt":"2013-07-18T20:02:12Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nThe sentinel function attribute is not understood by versions of\nthe gcc compiler prior to v4.0. At present, for earlier versions\nof gcc, the build issues 108 warnings related to the unknown\nattribute. In order to suppress the warnings, we conditionally\ndefine the GIT_SENTINEL macro to provide the sentinel attribute\nfor gcc v4.0 and newer.\n\nSigned-off-by: Ramsay Jones \n---\n\nThis was built on the next branch\n\nATB,\nRamsay Jones\n\n argv-array.h | 2 +-\n builtin/revert.c | 4 ++--\n exec_cmd.h | 2 +-\n git-compat-util.h | 7 +++++++\n run-command.h | 2 +-\n 5 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/argv-array.h b/argv-array.h\nindex e805748..9a4c053 100644\n--- a/argv-array.h\n+++ b/argv-array.h\n@@ -15,7 +15,7 @@ void argv_array_init(struct argv_array *);\n void argv_array_push(struct argv_array *, const char *);\n __attribute__((format (printf,2,3)))\n void argv_array_pushf(struct argv_array *, const char *fmt, ...);\n-__attribute__((sentinel))\n+GIT_SENTINEL(0)\n void argv_array_pushl(struct argv_array *, ...);\n void argv_array_pop(struct argv_array *);\n void argv_array_clear(struct argv_array *);\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex b8b5174..219f354 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -54,7 +54,7 @@ static int option_parse_x(const struct option *opt,\n \treturn 0;\n }\n \n-__attribute__((sentinel))\n+GIT_SENTINEL(0)\n static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n {\n \tconst char *this_opt;\n@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n \t\tdie(_(\"%s: %s cannot be used with %s\"), me, this_opt, base_opt);\n }\n \n-__attribute__((sentinel))\n+GIT_SENTINEL(0)\n static void verify_opt_mutually_compatible(const char *me, ...)\n {\n \tconst char *opt1, *opt2 = NULL;\ndiff --git a/exec_cmd.h b/exec_cmd.h\nindex 307b55c..fbbb44d 100644\n--- a/exec_cmd.h\n+++ b/exec_cmd.h\n@@ -7,7 +7,7 @@ extern const char *git_exec_path(void);\n extern void setup_path(void);\n extern const char **prepare_git_cmd(const char **argv);\n extern int execv_git_cmd(const char **argv); /* NULL terminated */\n-__attribute__((sentinel))\n+GIT_SENTINEL(0)\n extern int execl_git_cmd(const char *cmd, ...);\n extern const char *system_path(const char *path);\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ff193f4..cac8157 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -303,6 +303,13 @@ extern char *gitbasename(char *);\n #endif\n #endif\n \n+/* The sentinel attribute is valid from gcc version 4.0 */\n+#if defined(__GNUC__) && (__GNUC__ >= 4)\n+#define GIT_SENTINEL(n) __attribute__((sentinel(n)))\n+#else\n+#define GIT_SENTINEL(n)\n+#endif\n+\n #include \"compat/bswap.h\"\n \n #ifdef USE_WILDMATCH\ndiff --git a/run-command.h b/run-command.h\nindex 0a47679..2f2b453 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -46,7 +46,7 @@ int finish_command(struct child_process *);\n int run_command(struct child_process *);\n \n extern char *find_hook(const char *name);\n-__attribute__((sentinel))\n+GIT_SENTINEL(0)\n extern int run_hook(const char *index_file, const char *name, ...);\n \n #define RUN_COMMAND_NO_STDIN 1\n-- \n1.8.3\n"},{"id":"223695","messageId":"20130718203150.GA15735@sigill.intra.peff.net","threadId":"34476","inReplyTo":"51E849C4.7020305@ramsay1.demon.co.uk","subject":"Re: [PATCH] Add the GIT_SENTINEL macro","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-18T20:31:51Z","receivedAt":"2013-07-18T20:31:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 18, 2013 at 09:02:12PM +0100, Ramsay Jones wrote:\n\n> The sentinel function attribute is not understood by versions of\n> the gcc compiler prior to v4.0. At present, for earlier versions\n> of gcc, the build issues 108 warnings related to the unknown\n> attribute. In order to suppress the warnings, we conditionally\n> define the GIT_SENTINEL macro to provide the sentinel attribute\n> for gcc v4.0 and newer.\n> \n> Signed-off-by: Ramsay Jones \n> ---\n\nAcked-by: Jeff King \n\n> -__attribute__((sentinel))\n> +GIT_SENTINEL(0)\n> void argv_array_pushl(struct argv_array *, ...);\n\nWe could also add GIT_SENTINEL to handle the default-zero case, but I do\nnot think it is worth the trouble.\n\n-Peff\n"},{"id":"223702","messageId":"20130718210652.GX14690@google.com","threadId":"34476","inReplyTo":"51E849C4.7020305@ramsay1.demon.co.uk","subject":"Re: [PATCH] Add the GIT_SENTINEL macro","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-18T21:06:52Z","receivedAt":"2013-07-18T21:06:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramsay Jones wrote:\n\n> This was built on the next branch\n\nAll the uses of the sentinel attribute seem to come from\neccb6149 (use \"sentinel\" function attribute for variadic\nlists, 2013-07-09), so this should be okay to go on top of the\njk/gcc-function-attributes branch.\n\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -303,6 +303,13 @@ extern char *gitbasename(char *);\n> #endif\n> #endif\n> \n> +/* The sentinel attribute is valid from gcc version 4.0 */\n> +#if defined(__GNUC__) && (__GNUC__ >= 4)\n> +#define GIT_SENTINEL(n) __attribute__((sentinel(n)))\n> +#else\n> +#define GIT_SENTINEL(n)\n> +#endif\n\nI'd mildly prefer\n\n\t#if ...\n\t#define GIT_SENTINEL __attribute__((sentinel))\n\t#else\n\t...\n\n(without the numeric parameter). I don't know any function in git\n(or any other project for that matter) that takes extra parameters\nafter the NULL sentinel.\n\nBut I don't care much, so\n\nReviewed-by: Jonathan Nieder \n\nThanks for a pleasant patch.\nJonathan\n"},{"id":"223732","messageId":"7vr4evz0oc.fsf@alter.siamese.dyndns.org","threadId":"34476","inReplyTo":"51E849C4.7020305@ramsay1.demon.co.uk","subject":"Re: [PATCH] Add the GIT_SENTINEL macro","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-19T03:30:27Z","receivedAt":"2013-07-19T03:30:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> The sentinel function attribute is not understood by versions of\n> the gcc compiler prior to v4.0. At present, for earlier versions\n> of gcc, the build issues 108 warnings related to the unknown\n> attribute. In order to suppress the warnings, we conditionally\n> define the GIT_SENTINEL macro to provide the sentinel attribute\n> for gcc v4.0 and newer.\n>\n> Signed-off-by: Ramsay Jones \n> ---\n>\n> This was built on the next branch\n\nIt seems to apply well on the tip of jk/gcc-function-attributes.\n\n\n - This macro is not about \"git\" at all, so I'll edit the patch to\n call it GCC_ATTR_SENTINEL before applying.\n\n - Also I'll drop the (0) at the end.\n\nThanks.\n"},{"id":"223734","messageId":"20130719033634.GB27853@google.com","threadId":"34476","inReplyTo":"7vr4evz0oc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add the GIT_SENTINEL macro","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-19T03:36:34Z","receivedAt":"2013-07-19T03:36:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> It seems to apply well on the tip of jk/gcc-function-attributes.\n>\n> - This macro is not about \"git\" at all, so I'll edit the patch to\n> call it GCC_ATTR_SENTINEL before applying.\n\nWould naming it something like LAST_ARG_MUST_BE_NULL instead make\nsense? That way, if some other compiler gains a different syntax for\nthe same annotation, it would be possible to do\n\n\t#if defined(__GNUC__) && __GNUC__ >= 4\n\t# define LAST_ARG_MUST_BE_NULL __attribute__((sentinel))\n\t#elif defined(_MSC_VER) && _MSC_VER > 27\n\t# define LAST_ARG_MUST_BE_NULL __declspec(lastargnull)\n\t#else\n\t# define LAST_ARG_MUST_BE_NULL\n\t#endif\n"},{"id":"223737","messageId":"7vmwpjyq4a.fsf@alter.siamese.dyndns.org","threadId":"34476","inReplyTo":"20130719033634.GB27853@google.com","subject":"Re: [PATCH] Add the GIT_SENTINEL macro","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-19T07:18:29Z","receivedAt":"2013-07-19T07:18:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> It seems to apply well on the tip of jk/gcc-function-attributes.\n>>\n>> - This macro is not about \"git\" at all, so I'll edit the patch to\n>> call it GCC_ATTR_SENTINEL before applying.\n>\n> Would naming it something like LAST_ARG_MUST_BE_NULL instead make\n> sense? That way, if some other compiler gains a different syntax for\n> the same annotation, it would be possible to do\n>\n> \t#if defined(__GNUC__) && __GNUC__ >= 4\n> \t# define LAST_ARG_MUST_BE_NULL __attribute__((sentinel))\n> \t#elif defined(_MSC_VER) && _MSC_VER > 27\n> \t# define LAST_ARG_MUST_BE_NULL __declspec(lastargnull)\n> \t#else\n> \t# define LAST_ARG_MUST_BE_NULL\n> \t#endif\n\nI do like last-arg-must-be-null name for its descriptiveness; with\nthe example of NORETURN vs NORETURN_PTR, however, I am not quite\nconvinced that it would help reusing the same macro to different\ncompilers.\n\nI would say it is already sheer luck that GCC's __attribute__(())\nand MSC's __declspec() both come at the same place in the function\ndeclaration syntax, i.e. before the usual declaration. Your next\ncompiler may want the magic immediately before the closing semicolon\nof the declaration, for example.\n"},{"id":"223879","messageId":"51EAE146.2070309@ramsay1.demon.co.uk","threadId":"34476","inReplyTo":"7vr4evz0oc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Add the GIT_SENTINEL macro","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2013-07-20T19:13:10Z","receivedAt":"2013-07-20T19:13:10Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> The sentinel function attribute is not understood by versions of\n>> the gcc compiler prior to v4.0. At present, for earlier versions\n>> of gcc, the build issues 108 warnings related to the unknown\n>> attribute. In order to suppress the warnings, we conditionally\n>> define the GIT_SENTINEL macro to provide the sentinel attribute\n>> for gcc v4.0 and newer.\n>>\n>> Signed-off-by: Ramsay Jones \n>> ---\n>>\n>> This was built on the next branch\n> \n> It seems to apply well on the tip of jk/gcc-function-attributes.\n> \n> \n> - This macro is not about \"git\" at all, so I'll edit the patch to\n> call it GCC_ATTR_SENTINEL before applying.\n> \n> - Also I'll drop the (0) at the end.\n> \n> Thanks.\n\nYes, GCC_ATTR_SENTINEL is a better name, but I like LAST_ARG_MUST_BE_NULL\neven more! The final commit 9fe3edc4 (\"Add the LAST_ARG_MUST_BE_NULL macro\",\n18-07-2013) is much better than my patch and works beautifully.\n\nThanks Junio, Jeff and Jonathan!\n\nATB,\nRamsay Jones\n"}]}