Make our JSON APIs a public interface sd-json.h#32628
Merged
poettering merged 5 commits intosystemd:mainfrom Jun 13, 2024
Merged
Make our JSON APIs a public interface sd-json.h#32628poettering merged 5 commits intosystemd:mainfrom
poettering merged 5 commits intosystemd:mainfrom
Conversation
Member
Author
|
This is v257 material, in case this wasn't clear. |
github-actions
bot
added
udev
network
journal
nspawn
resolve
login
documentation
systemctl
build-system
tests
sd-boot/sd-stub/bootctl
coredump
sleep
meson
hibernate-resume
busctl
repart
userdb
tpm2
sysupdate
vmspawn
nsresource
mountfsd
please-review
yuwata
approved these changes
Jun 12, 2024
|
|
||
| _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_json_variant, sd_json_variant_unref); | ||
|
|
||
| const char *sd_json_variant_string(sd_json_variant *v); |
Member
There was a problem hiding this comment.
Not addressed, though not a big matter.
| int sd_json_variant_unbase64(sd_json_variant *v, void **ret, size_t *ret_size); | ||
| int sd_json_variant_unhex(sd_json_variant *v, void **ret, size_t *ret_size); | ||
|
|
||
| const char *sd_json_variant_type_to_string(sd_json_variant_type_t t); |
poettering
added a commit
to poettering/systemd
that referenced
this pull request
Jun 12, 2024
In some recent PRs (e.g. systemd#32628) I started to systematically name return parameters that shall only be initialized on failure (because they carry additional error meta information, such as the line/column number of parse failures or so). Let's make this official in the coding style.
The functions more or less do the same thing. Merge them. This makes json_dispatch_path() the common resulting implementation. it learnt: 1. Will reset the path to NULL if specified as null in JSON 2. Depending on the JSON_SAFE flag will insist on normalized path or not With this the two implementations are identical, except for the differences now toggable via JSON_SAFE flag
This is preparation for making our Varlink API a public API. Since our Varlink API is built on top of our JSON API we need to make that public first (it's a nice API, but JSON APIs there are already enough, this is purely about the Varlink angle). I made most of the json.h APIs public, and just placed them in sd-json.h. Sometimes I wasn't so sure however, since the underlying data structures would have to be made public too. If in doubt I didn#t risk it, and moved the relevant API to src/libsystemd/sd-json/json-util.h instead (without any sd_* symbol prefixes). This is mostly a giant search/replace patch.
The flag is fairly generic these days and just selects a slightly stricter validation, with details depending on the selected dispatch function. Hence, let's give it more precise name, in particular one that mirrors the SD_JSON_RELAXED flag nicely (which does the opposite: relaxes parsing)
poettering
added
good-to-merge/waiting-for-ci 👍
github-actions
bot
removed
good-to-merge/waiting-for-ci 👍
poettering
added a commit
to poettering/systemd
that referenced
this pull request
Jun 13, 2024
In some recent PRs (e.g. systemd#32628) I started to systematically name return parameters that shall only be initialized on failure (because they carry additional error meta information, such as the line/column number of parse failures or so). Let's make this official in the coding style.
poettering
added a commit
that referenced
this pull request
Jun 13, 2024
In some recent PRs (e.g. #32628) I started to systematically name return parameters that shall only be initialized on failure (because they carry additional error meta information, such as the line/column number of parse failures or so). Let's make this official in the coding style.
bluca
reviewed
Jun 13, 2024
| uint16_t depth; | ||
|
|
||
| JsonVariantType type:8; | ||
| int type:8; /* actually sd_json_variant_type_t, but reduced to 8bit size */ |
Member
There was a problem hiding this comment.
Bit field type of type int should have explicitly unsigned integral, explicitly signed integral, or enumeration type.
Member
There was a problem hiding this comment.
@poettering is this a false positive? Or does it need fixup?
Member
Author
There was a problem hiding this comment.
dunno, sd_json_variant_type_t is actually always going to be mapped to a signed int by the compiler. But of course we know that in this context it never can be negative, we are only going to store valid sd_json_variant_type_t in this field, i.e. not -EINVAL or so. Hence, the code as it is now is safe. I can understand the tools reasoning to cmplain abou this.
I'd probably say that it's a false positive, and leave the code as is.
Member
Author
There was a problem hiding this comment.
or maybe we can add the signed before it.
Member
There was a problem hiding this comment.
Can you give that a shot? It should become apparent in the PR if it is enough to fix it
poettering
added a commit
to poettering/systemd
that referenced
this pull request
Jun 13, 2024
poettering
added a commit
to poettering/systemd
that referenced
this pull request
Jun 13, 2024
poettering
added a commit
that referenced
this pull request
Jun 13, 2024
bluca
pushed a commit
to bluca/systemd
that referenced
this pull request
Jun 18, 2024
In some recent PRs (e.g. systemd#32628) I started to systematically name return parameters that shall only be initialized on failure (because they carry additional error meta information, such as the line/column number of parse failures or so). Let's make this official in the coding style. (cherry picked from commit 7811864)
keszybz
pushed a commit
that referenced
this pull request
Jun 18, 2024
In some recent PRs (e.g. #32628) I started to systematically name return parameters that shall only be initialized on failure (because they carry additional error meta information, such as the line/column number of parse failures or so). Let's make this official in the coding style. (cherry picked from commit 7811864)
jamacku
pushed a commit
to actions-private-playground/systemd
that referenced
this pull request
Jun 19, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is preparation for making our Varlink API a public API. Since our Varlink API is built on top of our JSON API we need to make that public first (it's a nice API, but JSON APIs there are already enough, this is purely about the Varlink angle).