Skip to content

Mark io target static variables as const to avoid race conditions#2557

Merged
fhanau merged 1 commit intomainfrom
felix/static-const-io
Aug 20, 2024
Merged

Mark io target static variables as const to avoid race conditions#2557
fhanau merged 1 commit intomainfrom
felix/static-const-io

Conversation

@fhanau
Copy link
Copy Markdown
Contributor

@fhanau fhanau commented Aug 20, 2024

Also cleans up remaining instances of uninitialized variables in that target. Some trivial cases of static variables elsewhere are also cleaned up.

  • In some cases using constexpr might be possible with little additional effort (in particular by making StringPtr constructors constexpr), but that was not the main objective here.

@fhanau fhanau requested a review from mikea August 20, 2024 01:45
@fhanau fhanau requested review from a team as code owners August 20, 2024 01:45
@fhanau fhanau requested a review from garrettgu10 August 20, 2024 01:45

struct EncDetail {
char* pass = &EMPTY_PASSPHRASE[0];
char* pass = const_cast<char*>(&EMPTY_PASSPHRASE[0]);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used const_cast before – let me know if this usage pattern is incorrect.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this case is ok since we don't expect anything to be written.

Also cleans up remaining instances of uninitialized variables in that
target.
@fhanau fhanau force-pushed the felix/static-const-io branch from a6951dc to c65a65a Compare August 20, 2024 01:57

// Class which regulates a SQL query, especially to control how queries created in JavaScript
// application code are handled.
class Regulator {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler insisted on Regulator being defined before the static constexpr Regulator variable, so I moved the subclass inside.

@fhanau fhanau merged commit bc35e3f into main Aug 20, 2024
@fhanau fhanau deleted the felix/static-const-io branch August 20, 2024 14:12
};

InputGate(Hooks& hooks = Hooks::DEFAULT);
InputGate(Hooks& hooks = const_cast<Hooks&>(Hooks::DEFAULT));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally any const_cast should come with a comment saying why it's OK. In this case: "const_cast OK because the object has no members, it's just a vtable."

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants