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
common: audit OPT_INT/OPT_UINT and handle incorrect signed uint #10835
Conversation
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
… only Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
Signed-off-by: Daniel Oliveira <doliveira@suse.com>
needs rebasing |
needs rebase |
@@ -480,7 +480,7 @@ int main(int argc, const char **argv) | |||
<< std::endl; | |||
exit(-err); | |||
} | |||
if (stats.avail_percent <= g_conf->mon_data_avail_crit) { | |||
if (static_cast<uint32_t>(stats.avail_percent) <= g_conf->mon_data_avail_crit) { |
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.
@oliveiradan Please keep PR title < 50 characters
https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#title-of-pull-requests-and-title-of-commits
@@ -946,83 +952,177 @@ int md_config_t::set_val_impl(const char *val, const config_option *opt) | |||
|
|||
int md_config_t::set_val_raw(const char *val, const config_option *opt) | |||
{ | |||
const int CEPH_SETTING_SAFE = 1; |
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.
@oliveiradan Why local variable is taken as caps?
Conventionally we take macros as this?
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.
@amitkumar50 , that was when I started working on Ceph (that's an old PR) and I used it as a way to identify a constant. However, as it ended up getting old and a rebase was needed, I ended up not touching it (once it is a pretty big change) 'till we were certain this PR would still be useful or not. Should this PR still be interesting, I will rebase and rework whatever is needed.
Thanks,
This is obsolete with the new options framework. What we probably want instead is to translate this into min/max values in options.cc |
Changes proposed to address issue: BUG #14934
http://tracker.ceph.com/issues/14934