You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
My superficial understanding of the issue is as follows: suppose you have a function that accepts a pointer, say *blob, and doesn't modify the stuff that pointer points at. Then it's fine to define your function to accept const foo *blob but then pass it in a non-const foo *blob from somewhere else, since const is supposed to mean "we don't change this here" not "we don't change this anywhere". However if it takes a pointer-to-a-pointer, say const foo **blob then there are ways that you'd actually be able to modify the contents being ultimately pointed at without, like, noticing. So it is not cool to pass in foo **blob.
So, I agree - seems like we should indeed remove the const.
According to this page the "most common solution" is to declare it like const foo* const *blob; however, I think this means you can't change the pointer *blob either and we do indeed need to do that in this function. And I tried it and it doesn't seem to work.
So here I've removed the const from **blob. This meant I also needed to remove const from **jsonand*metadata, since these are all pointing at the same chunk of memory.
I'm not sure what this means: "github-merge-queue bot removed this pull request from the merge queue due to no response for status checks". But it looks like it passed all checks, and the PR looks good to me. Can this merge be bumped somehow?
I suspect maaaaybe github's automated stuff dropped the ball? Trying again; will manually merge if not.
The auto-merge seems to have failed again. Maybe there's a time limit for all the checks to complete, and if they don't finish within that time limit the auto-merge gets cancelled? In which case that time limit needs to get extended somehow/somewhere, otherwise the auto-merge is seeming pretty useless. :-> Anyhow, manual merge for now, I guess?
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
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.
As discussed in #3425, it is maybe helpful to remove the
constfrom some of the arguments totsk_json_struct_metadata_get_blob.My superficial understanding of the issue is as follows: suppose you have a function that accepts a pointer, say
*blob, and doesn't modify the stuff that pointer points at. Then it's fine to define your function to acceptconst foo *blobbut then pass it in a non-constfoo *blobfrom somewhere else, sinceconstis supposed to mean "we don't change this here" not "we don't change this anywhere". However if it takes a pointer-to-a-pointer, sayconst foo **blobthen there are ways that you'd actually be able to modify the contents being ultimately pointed at without, like, noticing. So it is not cool to pass infoo **blob.So, I agree - seems like we should indeed remove the
const.According to this page the "most common solution" is to declare it like
const foo* const *blob; however, I think this means you can't change the pointer*blobeither and we do indeed need to do that in this function. And I tried it and it doesn't seem to work.So here I've removed the
constfrom**blob. This meant I also needed to removeconstfrom**jsonand*metadata, since these are all pointing at the same chunk of memory.