FIX: build warning - cast from pointer to integer of different size #110

ChromaticIsobar · 2021-08-09T08:58:30Z

See PR #92

While libm can be linked on generating final executable, it's still preferred to link it against the library itself to better represent the dependency hierarchy.

Added support for MSVC and (rather coincidentally) C++98 by replacing the decltype keyword and removing the corresponding preprocessor checks.

To prevent warnings in modern compilers

Example (starting in the source directory): $ mkdir build $ cd build $ ../configure $ make Make complains: ar rcs libjsonparser.a json.o ar: json.o: No such file or directory make: *** [Makefile:29: libjsonparser.a] Error 1

json.h: <inttypes.h> → <stdint.h> int64_t is defined in <stdint.h>, no need for the full <inttypes.h> json.h: <stdlib.h> → <stddef.h> <stddef.h> is enough to define size_t json.c: <stdlib.h> Now that <stdlib.h> has been removed from json.h, add it to json.c for malloc()/calloc() and free()

LB-- · 2021-08-10T02:23:25Z

Thanks! Unfortunately this may be more complicated than originally assumed. <stdint.h> isn't available until C99, and this library is targeting ANSI C, meaning that header won't always be available on all supported systems. Some other PRs are adjusting which headers are included in which places as well, such as #109, so perhaps once that change is merged it will be safe to rebase this and merge it.

DimitriPapadopoulos · 2021-08-10T06:56:41Z

Note that the library already includes header <inttypes.h> if json_int_t is not defined:

#ifndef json_int_t
   #ifndef _MSC_VER
      #include <inttypes.h>

and <stdint.h> on Windows:

#ifdef _MSC_VER
   #ifndef _CRT_SECURE_NO_WARNINGS
      #define _CRT_SECURE_NO_WARNINGS
   #endif
   #include <stdint.h>

Both headers are C99, not ANSI-C. But yes, it's quite possible to define json_int_t outside the library, so I agree the library is still ANSI C, except for a couple platform-dependent portability tricks.

DimitriPapadopoulos · 2021-08-10T10:32:54Z

Note that function json_alloc expects unsigned long anyway:

static void * json_alloc (json_state * state, unsigned long size, int zero)

Eventually you shall cast to unsigned long.

This tells me something's wrong here. I might be missing something, but why would the size of the allocation be a multiple of the value of a pointer?

            values_size = sizeof (*value->u.object.values) * value->u.object.length;

            if (! (value->u.object.values = (json_object_entry *) json_alloc
                  (state, values_size + ((unsigned long) value->u.object.values), 0)) )

It should be a multiple of the size of a pointer! For example, for array we simply have:

            if (! (value->u.array.values = (json_value **) json_alloc
               (state, value->u.array.length * sizeof (json_value *), 0)) )

jamesamcl · 2021-08-10T11:23:55Z

It's because there is something hacky in there. In the first pass, the values pointer field is used as an integer to store the length of all of the object names together. You can see this on line 423:

                 if (state.first_pass)
                    (*(json_char **) &top->u.object.values) += string_length + 1;
                 else
                 {  
                    top->u.object.values [top->u.object.length].name
                       = (json_char *) top->_reserved.object_mem;

                    top->u.object.values [top->u.object.length].name_length
                       = string_length;

                    (*(json_char **) &top->_reserved.object_mem) += string_length + 1;
                 }

Weirdly it casts it to a json_char* to add to it rather than an unsigned long or a size_t. I guess that's just a mistake. Then, in the second pass, it interprets it as an unsigned long to allocate enough space for all of the names together, in the fragment of code you found:

        values_size = sizeof (*value->u.object.values) * value->u.object.length;

        if (! (value->u.object.values = (json_object_entry *) json_alloc


              (state, values_size ///// <-- enough memory for the json_value objects


                 +


             ((unsigned long) value->u.object.values ///// <-- enough memory for the names of the objects


          ), 0)) )

The explicit way to describe what is happening would be with a union:

union {
    json_object_entry *values;
    size_t _reserved_names_len;
}

I guess I didn't do that because in ANSI C unions have to be named, so json->u.object.values would have to become json->u.object.u.values or something.

So to fix this I think the options would be:

Make it a union in json_value - makes the public API ugly
Add another reserved field to json_value for the names length. This would increase the size of every json_value by 1 pointer.
Make a json_value_internal defined in json.c which is binary compatible with the public json_value, but with the union.
Create a macro for reinterpreting the pointer field as a size
Keep the kludge and fix the casting somehow

I think option 3 is the cleanest? But any suggestions welcome.

DimitriPapadopoulos · 2021-08-11T19:13:13Z

Thanks for the explanation. It's probably difficult to totally remove the warning when retrieving an integer from a larger pointer, unless a union is used to store the different types.

I do not find 1 that bad, especially since you have been using the very neat (short) name u for unions. But it does expose a field that is used only internally, if I understand you correctly.
Option 5 looks good, but I have no clue how to achieve it.
Option 4 probably won't help, a macro cannot remove a warning that option 5 cannot remove.
If the fields of json_value are not used at the same time, option 2 will use extra space for no reason.
If I understand you correctly, option 3 is similar to 1, except the extra union will not be visible in the API. json_value_internal * will be cast to json_value * and back. Best option indeed.

…whitespaces Get rid of trailing whitespaces

…signed_int cur_line and cur_col are unsigned

Added fall through markers

example: fix typo

Remove dependency on decltype in C++ API

Add LDFLAGS support and missing lm option

ChromaticIsobar · 2021-08-28T09:13:02Z

@DimitriPapadopoulos I'll be away from work for a week, so I won't be able to answer before the 6th of September

LB-- · 2021-09-03T02:03:32Z

How to reproduce the compiler warning?

I do not see this warning on a 64-bit Ubuntu 18.04 machine, even with -pendatic or -Wall:
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

Do I need a 32-bit machine?

The warning only occurs when compiling for 64-bit architectures (e.g. x86_64 or ARM64) when unsigned long is 32-bits instead of 64-bits. So, typically you would encounter this warning when targeting Windows. For example, MSVC has C4311 enabled by default: https://gcc.godbolt.org/z/8Yo4Pjc3f

warning C4311: 'type cast': pointer truncation from 'void *' to 'unsigned long'

To get a similar warning for clang or gcc you would have to target Windows 64-bit as that is what causes unsigned long to be 32-bits while pointers are 64-bits, and you may have to explicitly enable -Wshorten-64-to-32.

ChromaticIsobar · 2021-09-09T13:01:45Z

I confirm what @LB-- said: this warning occurs when targeting 64-bit Windows.
I can reproduce the warning on my Ubuntu 16.04 machine using the x86_64-w64-mingw32-gcc compiler (version 5.3.1)

DimitriPapadopoulos · 2021-09-09T13:06:14Z

Thank you @ChromaticIsobar. I won't have time to cross-compile to test that in the near future, could you perhaps check whether #133 helps in your case?

ChromaticIsobar · 2021-09-09T13:16:10Z

It does. It took a little because #133 caused stdlib.h not to be included and I had to add it in all sources that needed it.
But now it does compile!

DimitriPapadopoulos · 2021-09-09T13:23:57Z

Do I get it right that <stdlib.h> is required by your own sources, not by json-parser itself?

DimitriPapadopoulos · 2021-09-09T13:27:13Z

And actually this change has not removed <stdlib.h>, has it?

ChromaticIsobar · 2021-09-09T13:39:41Z

Do I get it right that <stdlib.h> is required by your own sources, not by json-parser itself?

That's correct

Before #133 I included json.h, which in turn included stdlib.h
Now, json.h doesn't include stdlib.h anymore (but it includes stddef.h)

https://github.com/DimitriPapadopoulos/json-parser/blame/5a1fc1442a01ac5f4471931a59b8bee799f30c32/json.h#L47

So, I had to add the #include in my sources

DimitriPapadopoulos · 2021-09-09T15:55:30Z

Right, while you should always include <stdlib.h> yourself if you use definitions from that header, that's sort of a regression. Yet I am in favour of that regression, because the typical use is programmers including json.c and json.h directly in their projects, so that's something they should be able to handle, like you.

ChromaticIsobar · 2021-09-09T16:27:00Z

I totally agree

ChromaticIsobar · 2021-09-14T09:16:02Z

Do you want me to close this PR in favour of #133 ?

File test.py is executable in Git, yet does not start with "#!".

…-parser#145

Forgot to merge this with the others...

LB-- · 2021-10-30T22:19:32Z

My plan currently is this:

For v1.1.1, I will merge this PR with slight changes so that uintptr_t is only used if it's available, and otherwise the original code remains.
For v1.2.0, this will be obsoleted by Build warning - cast from pointer to integer of different size #133

The goal is to at least silence the warning for most people ASAP before engaging in a more drastic change for the sake of silencing the warning in C89. Any objections to this plan?

That's the standard over all the C standard library.

Either it has been defined explictly, or we attempt to calculate it using the method reverted by fcfa748. See discussion in json-parser#84 and json-parser#102.

Fix valid-0013.json added by fab533e (json-parser#125)

It looks like a bad idea to compare apply to a double operators ++ and --, or to compare to integers.

…-parser#144

LB-- · 2021-11-03T00:13:54Z

Oops. Well, the PR is a bit of a mess now, but the relevant changes can be seen in b10ff61 - I guess now it just handles the edge case where size_t and uintptr_t aren't the same size...

zackp30 and others added 15 commits May 5, 2016 15:55

Fix Pebble compilation (json-parser#49)

e9dabc9

example: fix typo

46a1047

Add LDFLAGS support and missing lm option

9cf18e3

While libm can be linked on generating final executable, it's still preferred to link it against the library itself to better represent the dependency hierarchy.

Enable C++ iterators in MSVC (and C++98)

1d56c07

Added support for MSVC and (rather coincidentally) C++98 by replacing the decltype keyword and removing the corresponding preprocessor checks.

Added fall through markers

0ecf2d7

To prevent warnings in modern compilers

Get rid of trailing whitespaces

8594d7d

./configure complains about ignored datarootdir variable

055ab42

Remove unnecessary/problematic AC_ macro

fd7101a

Enable the JSON_TRACK_SOURCE functionality from the configure script

a7cedd7

Generate new version of the configure script

f80e44e

Can not successfully build from a different directory

ee4103b

Example (starting in the source directory): $ mkdir build $ cd build $ ../configure $ make Make complains: ar rcs libjsonparser.a json.o ar: json.o: No such file or directory make: *** [Makefile:29: libjsonparser.a] Error 1

Get rid of trailing empty lines

10f4a79

cur_line and cur_col are unsigned

159db39

FIX: build error - cast from pointer to integer of different size

9083578

LB-- added 8 commits August 13, 2021 19:33

Merge pull request json-parser#100 from DimitriPapadopoulos/trailing_…

c75e8cd

…whitespaces Get rid of trailing whitespaces

Merge branch 'master' into printf_unsigned_int

7545b67

Merge pull request json-parser#108 from DimitriPapadopoulos/printf_un…

d96b368

…signed_int cur_line and cur_col are unsigned

Merge pull request json-parser#88 from dzavalishin/master

4a7d6e3

Added fall through markers

Merge pull request json-parser#65 from tildearrow/master

08f6281

example: fix typo

Merge pull request json-parser#77 from martinschmauder/master

a58d231

Remove dependency on decltype in C++ API

Add CI step for running configure script and make

b4efb6c

Merge json-parser#104, json-parser#105, and json-parser#107

7c16f37

LB-- added this to the v1.1.1 milestone Aug 14, 2021

Merge pull request json-parser#75 from yangfl/master

76f553b

Add LDFLAGS support and missing lm option

DimitriPapadopoulos added 2 commits August 28, 2021 15:49

Use existing C89 header files

13faf02

include first, definitions later

67ceb35

LB-- self-assigned this Sep 3, 2021

Fix flake8 alerts

5b44508

DimitriPapadopoulos and others added 3 commits September 15, 2021 08:11

Executable file should be executable

de022c7

File test.py is executable in Git, yet does not start with "#!".

Merge PRs json-parser#127, json-parser#130, json-parser#141, and json…

bb4135c

…-parser#145

Merge PR json-parser#140

3c6d74e

Forgot to merge this with the others...

DimitriPapadopoulos and others added 9 commits October 31, 2021 09:16

Use size_t for memory size

ad3a2e6

That's the standard over all the C standard library.

<stdint.h> is available only in C99 or later

fa11f53

Define JSON_INT_MAX

7b12984

Either it has been defined explictly, or we attempt to calculate it using the method reverted by fcfa748. See discussion in json-parser#84 and json-parser#102.

Add more checks

837883f

Valid example file was invalid!

b4bb0cc

Fix valid-0013.json added by fab533e (json-parser#125)

num_digits: double → int

8524529

It looks like a bad idea to compare apply to a double operators ++ and --, or to compare to integers.

Merge PRs json-parser#126, json-parser#136, json-parser#139, and json…

b9b5d53

…-parser#144

Merge json-parser#115 and add documentation

24df9ad

Merge PR json-parser#110 with uintptr_t detection

b10ff61

LB-- merged commit b10ff61 into json-parser:master Nov 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: build warning - cast from pointer to integer of different size #110

FIX: build warning - cast from pointer to integer of different size #110

ChromaticIsobar commented Aug 9, 2021

LB-- commented Aug 10, 2021

DimitriPapadopoulos commented Aug 10, 2021

DimitriPapadopoulos commented Aug 10, 2021 •

edited

jamesamcl commented Aug 10, 2021

DimitriPapadopoulos commented Aug 11, 2021 •

edited

ChromaticIsobar commented Aug 28, 2021

LB-- commented Sep 3, 2021 •

edited

ChromaticIsobar commented Sep 9, 2021

DimitriPapadopoulos commented Sep 9, 2021

ChromaticIsobar commented Sep 9, 2021 •

edited

DimitriPapadopoulos commented Sep 9, 2021

DimitriPapadopoulos commented Sep 9, 2021

ChromaticIsobar commented Sep 9, 2021

DimitriPapadopoulos commented Sep 9, 2021 •

edited

ChromaticIsobar commented Sep 9, 2021

ChromaticIsobar commented Sep 14, 2021

LB-- commented Oct 30, 2021

LB-- commented Nov 3, 2021

FIX: build warning - cast from pointer to integer of different size #110

FIX: build warning - cast from pointer to integer of different size #110

Conversation

ChromaticIsobar commented Aug 9, 2021

LB-- commented Aug 10, 2021

DimitriPapadopoulos commented Aug 10, 2021

DimitriPapadopoulos commented Aug 10, 2021 • edited

jamesamcl commented Aug 10, 2021

DimitriPapadopoulos commented Aug 11, 2021 • edited

ChromaticIsobar commented Aug 28, 2021

LB-- commented Sep 3, 2021 • edited

ChromaticIsobar commented Sep 9, 2021

DimitriPapadopoulos commented Sep 9, 2021

ChromaticIsobar commented Sep 9, 2021 • edited

DimitriPapadopoulos commented Sep 9, 2021

DimitriPapadopoulos commented Sep 9, 2021

ChromaticIsobar commented Sep 9, 2021

DimitriPapadopoulos commented Sep 9, 2021 • edited

ChromaticIsobar commented Sep 9, 2021

ChromaticIsobar commented Sep 14, 2021

LB-- commented Oct 30, 2021

LB-- commented Nov 3, 2021

DimitriPapadopoulos commented Aug 10, 2021 •

edited

DimitriPapadopoulos commented Aug 11, 2021 •

edited

LB-- commented Sep 3, 2021 •

edited

ChromaticIsobar commented Sep 9, 2021 •

edited

DimitriPapadopoulos commented Sep 9, 2021 •

edited