{"thread":{"id":"28362","subject":"[PATCH 2/2] obstack.c: Fix some sparse warnings","startedAt":"2011-09-11T19:26:53Z","lastAt":"2011-09-13T23:18:55Z","messageCount":3,"participants":["Ramsay Jones","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"175305","messageId":"4E6D0B7D.9070602@ramsay1.demon.co.uk","threadId":"28362","inReplyTo":null,"subject":"[PATCH 2/2] obstack.c: Fix some sparse warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-09-11T19:26:53Z","receivedAt":"2011-09-11T19:26:53Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nIn particular, sparse issues the following warnings:\n\n compat/obstack.c:176:17: warning: Using plain integer as NULL pointer\n compat/obstack.c:224:17: warning: Using plain integer as NULL pointer\n compat/obstack.c:324:16: warning: Using plain integer as NULL pointer\n compat/obstack.c:329:16: warning: Using plain integer as NULL pointer\n compat/obstack.c:347:16: warning: Using plain integer as NULL pointer\n compat/obstack.c:362:19: warning: Using plain integer as NULL pointer\n compat/obstack.c:379:29: warning: Using plain integer as NULL pointer\n compat/obstack.c:399:1: error: symbol 'print_and_abort' redeclared with \\\n different type (originally declared at compat/obstack.c:95) \\\n - different modifiers\n\nSigned-off-by: Ramsay Jones \n---\n compat/obstack.c | 15 +++++++--------\n 1 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/compat/obstack.c b/compat/obstack.c\nindex a89ab5b..e276ccd 100644\n--- a/compat/obstack.c\n+++ b/compat/obstack.c\n@@ -173,7 +173,7 @@ _obstack_begin (struct obstack *h,\n \t\t\t\t\t alignment - 1);\n h->chunk_limit = chunk->limit\n = (char *) chunk + h->chunk_size;\n- chunk->prev = 0;\n+ chunk->prev = NULL;\n /* The initial chunk now contains no empty object. */\n h->maybe_empty_object = 0;\n h->alloc_failed = 0;\n@@ -221,7 +221,7 @@ _obstack_begin_1 (struct obstack *h, int size, int alignment,\n \t\t\t\t\t alignment - 1);\n h->chunk_limit = chunk->limit\n = (char *) chunk + h->chunk_size;\n- chunk->prev = 0;\n+ chunk->prev = NULL;\n /* The initial chunk now contains no empty object. */\n h->maybe_empty_object = 0;\n h->alloc_failed = 0;\n@@ -321,12 +321,12 @@ _obstack_allocated_p (struct obstack *h, void *obj)\n /* We use >= rather than > since the object cannot be exactly at\n the beginning of the chunk but might be an empty object exactly\n at the end of an adjacent chunk. */\n- while (lp != 0 && ((void *) lp >= obj || (void *) (lp)->limit < obj))\n+ while (lp != NULL && ((void *) lp >= obj || (void *) (lp)->limit < obj))\n {\n plp = lp->prev;\n lp = plp;\n }\n- return lp != 0;\n+ return lp != NULL;\n }\n \f\n /* Free objects in obstack H, including OBJ and everything allocate\n@@ -344,7 +344,7 @@ obstack_free (struct obstack *h, void *obj)\n /* We use >= because there cannot be an object at the beginning of a chunk.\n But there can be an empty object at that address\n at the end of another chunk. */\n- while (lp != 0 && ((void *) lp >= obj || (void *) (lp)->limit < obj))\n+ while (lp != NULL && ((void *) lp >= obj || (void *) (lp)->limit < obj))\n {\n plp = lp->prev;\n CALL_FREEFUN (h, lp);\n@@ -359,7 +359,7 @@ obstack_free (struct obstack *h, void *obj)\n h->chunk_limit = lp->limit;\n h->chunk = lp;\n }\n- else if (obj != 0)\n+ else if (obj != NULL)\n /* obj is not in any of the chunks! */\n abort ();\n }\n@@ -376,7 +376,7 @@ _obstack_memory_used (struct obstack *h)\n register struct _obstack_chunk* lp;\n register int nbytes = 0;\n \n- for (lp = h->chunk; lp != 0; lp = lp->prev)\n+ for (lp = h->chunk; lp != NULL; lp = lp->prev)\n {\n nbytes += lp->limit - (char *) lp;\n }\n@@ -395,7 +395,6 @@ _obstack_memory_used (struct obstack *h)\n # endif\n \n static void\n-__attribute__ ((noreturn))\n print_and_abort (void)\n {\n /* Don't change any of these strings. Yes, it would be possible to add\n-- \n1.7.6\n"},{"id":"175320","messageId":"CAGdFq_ifU2WWbCpRY_EFY=_hwwtFs0eqMhJ7sRoUhrivABFoFw@mail.gmail.com","threadId":"28362","inReplyTo":"4E6D0B7D.9070602@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/2] obstack.c: Fix some sparse warnings","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-09-11T21:56:43Z","receivedAt":"2011-09-11T21:56:43Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Sun, Sep 11, 2011 at 21:26, Ramsay Jones wrote:\n>    compat/obstack.c:399:1: error: symbol 'print_and_abort' redeclared with \\\n>        different type (originally declared at compat/obstack.c:95) \\\n>        - different modifiers\n\n> @@ -395,7 +395,6 @@ _obstack_memory_used (struct obstack *h)\n>  # endif\n>\n>  static void\n> -__attribute__ ((noreturn))\n>  print_and_abort (void)\n>  {\n>   /* Don't change any of these strings.  Yes, it would be possible to add\n\nWouldn't the better solution be to add noreturn to the declaration at\ncompat/obstack.c:95?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"175438","messageId":"4E6FE4DF.7060200@ramsay1.demon.co.uk","threadId":"28362","inReplyTo":"CAGdFq_ifU2WWbCpRY_EFY=_hwwtFs0eqMhJ7sRoUhrivABFoFw@mail.gmail.com","subject":"Re: [PATCH 2/2] obstack.c: Fix some sparse warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-09-13T23:18:55Z","receivedAt":"2011-09-13T23:18:55Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Sverre Rabbelier wrote:\n> Heya,\n> \n> On Sun, Sep 11, 2011 at 21:26, Ramsay Jones wrote:\n>> compat/obstack.c:399:1: error: symbol 'print_and_abort' redeclared with \\\n>> different type (originally declared at compat/obstack.c:95) \\\n>> - different modifiers\n> \n>> @@ -395,7 +395,6 @@ _obstack_memory_used (struct obstack *h)\n>> # endif\n>>\n>> static void\n>> -__attribute__ ((noreturn))\n>> print_and_abort (void)\n>> {\n>> /* Don't change any of these strings. Yes, it would be possible to add\n> \n> Wouldn't the better solution be to add noreturn to the declaration at\n> compat/obstack.c:95?\n\nHmm, well ... maybe; it is at least debatable. But I decided no! :-D\n\nFirst, although I would not dismiss the possibility of some optimization\nof the code of print_and_abort() (the *callee*), the main benefit of the\nnoreturn attribute should in fact be at the call sites (ie the *caller*).\nSo, yes, in general, the declaration of the function should have the\nnoreturn attribute applied, in addition to the definition, in order to\nallow the compiler to apply some optimizations to the call sites.\n\n[Note, also, that we should use the NORETURN and NORETURN_PTR macros.]\n\nIn this case, however, there are no (direct) call sites. This function\nwould only be called indirectly via the 'obstack_alloc_failed_handler'\nfunction pointer. So, this would require the use of NORETURN_PTR on\nthat function pointer. In order to keep both the compiler(s) and sparse\nhappy, the required change would look like the diff given at the end\nof this mail.\n\nThis would work fine, and I would happily change the patch to include\nthis if it is deemed the better approach. However, I looked at the\ncall sites in _obstack_begin[_1](), and _obstack_newchunck() and could\nnot see any great opportunity for optimizing the code ... so I decided\nto go for the simpler patch ...\n\nATB,\nRamsay Jones\n\n-- >8 --\ndiff --git a/compat/obstack.c b/compat/obstack.c\nindex a89ab5b..2029b8f 100644\n--- a/compat/obstack.c\n+++ b/compat/obstack.c\n@@ -92,8 +92,8 @@ enum\n abort gracefully or use longjump - but shouldn't return. This\n variable by default points to the internal function\n `print_and_abort'. */\n-static void print_and_abort (void);\n-void (*obstack_alloc_failed_handler) (void) = print_and_abort;\n+static void NORETURN print_and_abort (void);\n+NORETURN_PTR void (*obstack_alloc_failed_handler) (void) = print_and_abort;\n \n # ifdef _LIBC\n # if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_3_4)\n@@ -395,7 +395,7 @@ _obstack_memory_used (struct obstack *h)\n # endif\n \n static void\n-__attribute__ ((noreturn))\n+NORETURN\n print_and_abort (void)\n {\n /* Don't change any of these strings. Yes, it would be possible to add\ndiff --git a/compat/obstack.h b/compat/obstack.h\nindex d178bd6..122f93f 100644\n--- a/compat/obstack.h\n+++ b/compat/obstack.h\n@@ -194,7 +194,7 @@ void obstack_free (struct obstack *, void *);\n more memory. This can be set to a user defined function which\n should either abort gracefully or use longjump - but shouldn't\n return. The default action is to print a message and abort. */\n-extern void (*obstack_alloc_failed_handler) (void);\n+extern NORETURN_PTR void (*obstack_alloc_failed_handler) (void);\n \f\n /* Pointer to beginning of object being allocated or to be allocated next.\n Note that this might not be the final address of the object\n"}]}