Skip to content

Conversation

@smowton
Copy link
Contributor

@smowton smowton commented Jul 2, 2021

Otherwise because this dataflow instance populates AdditionalTaintStep there is an ever-present danger that a user will stumble into creating a recursive configuration, or at least that by using DataFlow5::Configuration for any other purpose they will needlessly recalculate the Serializability dataflow results.

@smowton smowton requested a review from a team as a code owner July 2, 2021 13:50
@smowton smowton added the no-change-note-required This PR does not need a change note label Jul 2, 2021
@github-actions github-actions bot added the Java label Jul 2, 2021
@aschackmull
Copy link
Contributor

Is number 5 used for anything else at the moment? Otherwise we could consider just renaming it (and possibly renaming number 6 as that is also for a reserved use case IIRC). We have plenty of copies already, so might as well try to keep the number down.

@smowton
Copy link
Contributor Author

smowton commented Jul 16, 2021

They do all have other users, though few in number.

@aschackmull
Copy link
Contributor

LGTM, but needs resync.

smowton and others added 3 commits August 3, 2021 10:36
Otherwise because this dataflow instance populates AdditionalTaintStep there is an ever-present danger that a user will stumble into creating a recursive configuration, or at least that by using DataFlow5::Configuration for any other purpose they will needlessly recalculate the Serializability dataflow results.
Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
@smowton smowton force-pushed the smowton/admin/serializability-dataflow branch from e6d9c33 to afa8278 Compare August 3, 2021 09:36
@smowton
Copy link
Contributor Author

smowton commented Aug 3, 2021

Accepted suggestions and rebased

@aschackmull
Copy link
Contributor

CI still looks very red for some reason.

@smowton
Copy link
Contributor Author

smowton commented Aug 3, 2021

Looks like there were concurrent changes to DataFlowImpl.qll. Resynced.

@aschackmull aschackmull merged commit ad86641 into github:main Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants