Sitelet https://github.com/json-c/json-c/pull/740
Skip to content

Fix warnings of clang -Wshorten-64-to-32 - #740

Open
rouault wants to merge 1 commit into
json-c:masterfrom
rouault:fix_shorten-64-to-32
Open

rouault wants to merge 1 commit into
json-c:masterfrom
rouault:fix_shorten-64-to-32

Conversation

@rouault

@rouault rouault commented Jan 12, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@hawicz

hawicz commented Feb 19, 2022

Copy link
Copy Markdown
Member

I think at least some (all?) of these warnings are actual problems, and rather than papering over them by casting to int, we need to take adjust the types used. e.g. printbuf_memappend() should take a size_t, int ret should be size_t ret instead, etc...

That might break API/ABI compatibility, so I'll need to take a closer look at each change.

hawicz added a commit that referenced this pull request Jul 31, 2022
@hawicz

hawicz commented Jul 31, 2022

Copy link
Copy Markdown
Member

I fixed some of these issues in commit bdd5e03. Changes to json_tokener.c and json_object.c are still needed.

@hawicz

hawicz commented Jul 7, 2023

Copy link
Copy Markdown
Member

Fixing this means at least changing the size and bpos members of struct printbuf to be size_t.
That is an ABI breaking change, and that doesn't even include changing the size of various function parameters too, so actually fixing this will need to wait until we're ready for a 1.0 release.

@hawicz hawicz added the release-1.0 Features and issues for a potential 1.0 release label Jul 7, 2023
@hmh

hmh commented Apr 28, 2025

Copy link
Copy Markdown

If you're going to change the API/ABI, please do use size_t instead of int for every object-size-related parameter in the API, and please use const on parameters taken by reference that are not going to be modified by the function call (i.e. "const struct json_object *jo" if *jo is not going to be modified by that API function).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-1.0 Features and issues for a potential 1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants