DaveGamble / cJSON

Ultralightweight JSON parser in ANSI C
MIT License
10.76k stars 3.21k forks source link

Name conflict with "index" in cJSON.c and string.h #288

Open randygalbraith opened 6 years ago

randygalbraith commented 6 years ago

While compiling cJSON on Red Hat GNU/Linux 6 gcc reported a name conflict error. This happened only with enhanced gcc flags for diagnostics we typically use to compile packages. For what it is worth here is the patch that I applied to resolve the issue (see below). Thank you to all the contributors of cJSON. Our lead engineer who is using it has found it very nice.

Cheers, -Randy

--- cJSON.c.~1~
+++ cJSON.c
@@ -1740,7 +1740,7 @@ CJSON_PUBLIC(int) cJSON_GetArraySize(const cJSON *array)
     return (int)size;
 }

-static cJSON* get_array_item(const cJSON *array, size_t index)
+static cJSON* get_array_item(const cJSON *array, size_t index_cj)
 {
     cJSON *current_child = NULL;

@@ -1750,23 +1750,23 @@ static cJSON* get_array_item(const cJSON *array, size_t index)
     }

     current_child = array->child;
-    while ((current_child != NULL) && (index > 0))
+    while ((current_child != NULL) && (index_cj > 0))
     {
-        index--;
+        index_cj--;
         current_child = current_child->next;
     }

     return current_child;
 }

-CJSON_PUBLIC(cJSON *) cJSON_GetArrayItem(const cJSON *array, int index)
+CJSON_PUBLIC(cJSON *) cJSON_GetArrayItem(const cJSON *array, int index_cj)
 {
-    if (index < 0)
+    if (index_cj < 0)
     {
         return NULL;
     }

-    return get_array_item(array, (size_t)index);
+    return get_array_item(array, (size_t)index_cj);
 }

 static cJSON *get_object_item(const cJSON * const object, const char * const name, const cJSON_bool case_sensitive)
--- cJSON_Utils.c.~1~
+++ cJSON_Utils.c
@@ -251,7 +251,7 @@ static cJSON *get_array_item(const cJSON *array, size_t item)
     return child;
 }

-static cJSON_bool decode_array_index_from_pointer(const unsigned char * const pointer, size_t * const index)
+static cJSON_bool decode_array_index_from_pointer(const unsigned char * const pointer, size_t * const index_cj)
 {
     size_t parsed_index = 0;
     size_t position = 0;
@@ -273,7 +273,7 @@ static cJSON_bool decode_array_index_from_pointer(const unsigned char * const po
         return 0;
     }

-    *index = parsed_index;
+    *index_cj = parsed_index;

     return 1;
 }
@@ -293,13 +293,13 @@ static cJSON *get_item_from_pointer(cJSON * const object, const char * pointer,
         pointer++;
         if (cJSON_IsArray(current_element))
         {
-            size_t index = 0;
-            if (!decode_array_index_from_pointer((const unsigned char*)pointer, &index))
+            size_t index_cj = 0;
+            if (!decode_array_index_from_pointer((const unsigned char*)pointer, &index_cj))
             {
                 return NULL;
             }

-            current_element = get_array_item(current_element, index);
+            current_element = get_array_item(current_element, index_cj);
         }
         else if (cJSON_IsObject(current_element))
         {
@@ -430,12 +430,12 @@ static cJSON *detach_path(cJSON *object, const unsigned char *path, const cJSON_

     if (cJSON_IsArray(parent))
     {
-        size_t index = 0;
-        if (!decode_array_index_from_pointer(child_pointer, &index))
+        size_t index_cj = 0;
+        if (!decode_array_index_from_pointer(child_pointer, &index_cj))
         {
             goto cleanup;
         }
-        detached_item = detach_item_from_array(parent, index);
+        detached_item = detach_item_from_array(parent, index_cj);
     }
     else if (cJSON_IsObject(parent))
     {
@@ -960,14 +960,14 @@ static int apply_patch(cJSON *object, const cJSON *patch, const cJSON_bool case_
         }
         else
         {
-            size_t index = 0;
-            if (!decode_array_index_from_pointer(child_pointer, &index))
+            size_t index_cj = 0;
+            if (!decode_array_index_from_pointer(child_pointer, &index_cj))
             {
                 status = 11;
                 goto cleanup;
             }

-            if (!insert_item_in_array(parent, index, value))
+            if (!insert_item_in_array(parent, index_cj, value))
             {
                 status = 10;
                 goto cleanup;
@@ -1142,23 +1142,23 @@ static void create_patches(cJSON * const patches, const unsigned char * const pa

         case cJSON_Array:
         {
-            size_t index = 0;
+            size_t index_cj = 0;
             cJSON *from_child = from->child;
             cJSON *to_child = to->child;
             unsigned char *new_path = (unsigned char*)cJSON_malloc(strlen((const char*)path) + 20 + sizeof("/")); /* Allow space for 64bit int. log10(2^64) = 20 */

             /* generate patches for all array elements that exist in both "from" and "to" */
-            for (index = 0; (from_child != NULL) && (to_child != NULL); (void)(from_child = from_child->next), (void)(to_child = to_child->next), index++)
+            for (index_cj = 0; (from_child != NULL) && (to_child != NULL); (void)(from_child = from_child->next), (void)(to_child = to_child->next), index_cj++)
             {
                 /* check if conversion to unsigned long is valid
                  * This should be eliminated at compile time by dead code elimination
                  * if size_t is an alias of unsigned long, or if it is bigger */
-                if (index > ULONG_MAX)
+                if (index_cj > ULONG_MAX)
                 {
                     cJSON_free(new_path);
                     return;
                 }
-                sprintf((char*)new_path, "%s/%lu", path, (unsigned long)index); /* path of the current array element */
+                sprintf((char*)new_path, "%s/%lu", path, (unsigned long)index_cj); /* path of the current array element */
                 create_patches(patches, new_path, from_child, to_child, case_sensitive);
             }

@@ -1168,16 +1168,16 @@ static void create_patches(cJSON * const patches, const unsigned char * const pa
                 /* check if conversion to unsigned long is valid
                  * This should be eliminated at compile time by dead code elimination
                  * if size_t is an alias of unsigned long, or if it is bigger */
-                if (index > ULONG_MAX)
+                if (index_cj > ULONG_MAX)
                 {
                     cJSON_free(new_path);
                     return;
                 }
-                sprintf((char*)new_path, "%lu", (unsigned long)index);
+                sprintf((char*)new_path, "%lu", (unsigned long)index_cj);
                 compose_patch(patches, (const unsigned char*)"remove", path, new_path, NULL);
             }
             /* add new elements in 'to' that were not in 'from' */
-            for (; (to_child != NULL); (void)(to_child = to_child->next), index++)
+            for (; (to_child != NULL); (void)(to_child = to_child->next), index_cj++)
             {
                 compose_patch(patches, (const unsigned char*)"add", path, (const unsigned char*)"-", to_child);
             }
FSMaxB commented 6 years ago

I don't really understand why I shouldn't be allowed to name parameters index. Can you provide the error message and the diagnostic flags that you used to compile cJSON?

randygalbraith commented 6 years ago

Hi FSMaxB,

Sure. Here is the output:

gcc -c -fPIC -std=c89 -pedantic -Wall -Werror -Wstrict-prototypes -Wwrite-strings -Wshadow -Winit-self -Wcast-align -Wformat=2 -Wmissing-prototypes -Wstrict-overflow=2 -Wcast-qual -Wc++-compat -Wundef -Wswitch-default -Wconversion -O3 -pipe -std=gnu99 -fstack-protector cJSON.c cc1: warnings being treated as errors cJSON.c: In function 'get_array_item': cJSON.c:1743: error: declaration of 'index' shadows a global declaration /usr/include/string.h:489: error: shadowed declaration is here cJSON.c: In function 'cJSON_GetArrayItem': cJSON.c:1762: error: declaration of 'index' shadows a global declaration /usr/include/string.h:489: error: shadowed declaration is here make[1]: *** [Makefile:80: cJSON.o] Error 1

Please keep in mind this run of make and gcc is from within our internal package maintenance system. I suspect a run of make from the command line would not run into this issue.

Cheers, -Randy

arnout commented 6 years ago

man 3 index:

index, rindex - locate character in string #include <strings.h> char *index(const char *s, int c);

CONFORMING TO 4.3BSD; marked as LEGACY in POSIX.1-2001.

So this warning will occur whenever you include strings.h before cJSON.h.

Of course, this is perfectly correct C code with no risk at all, since these functions don't use the deprecated index() function. Combining -Werror with warnings not included in -Wall and -Wextra (like the -Wshadow that triggers this) is not a good idea.

FSMaxB commented 6 years ago

Sorry for taking so long to get back to this.

So if I understand this correctly, the index function comes from an outdated POSIX extension?

Then there maybe should a way for me to disable it by setting or unsetting a define. I'll look into that.

FSMaxB commented 6 years ago

@randygalbraith Can you check if the disable-nonstandard-Branch fixes this issue?