-
Notifications
You must be signed in to change notification settings - Fork 292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: remove tox_options_default(...) and make Tox_Options opaque #1825
base: master
Are you sure you want to change the base?
Conversation
tox_options_new(...) already returns a default initialized options struct and this function makes it impossible to malloc buffers in Tox_Options, so remove it.
9372c8d
to
6d9bd44
Compare
You should not malloc buffers in |
Reopening, resoning in: #909 (comment) Edit: @nurupo even if you leave out #909 removing this function still makes sense, since it provides no real value (reseting to defaults is done in |
5911606
to
81fe998
Compare
Updated this PR with the logical next step of making |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 19 of 19 files at r2, all commit messages.
Reviewable status: 0 of 1 approvals obtained (waiting on @sudden6)
toxcore/tox.h, line 520 at r2 (raw file):
/** * This struct contains all the startup options for Tox. You must tox_options_new to * allocate an object of this type and tox_options_free to free it.
"you must use"
and "and use"
Ok, I'm leaving #909 aside for now then. What is the uninitialized pointers problem? Are you referring back to #909 even though you said "if you leave out #909", or is there some other problem I don't know? Anyway, we do want to make |
81fe998
to
547330e
Compare
My mistake, after making I'd still remove @JFreegman thx, fixed. |
tox_options_new(...) already returns a default initialized options
struct and this function makes it impossible to malloc buffers in
Tox_Options, so remove it.
Required for #909
Fixes #273
This change is