Skip to content
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

Can SetLogger set severity to -1? #590

Closed
daohu527 opened this issue Oct 9, 2020 · 3 comments · Fixed by #1025
Closed

Can SetLogger set severity to -1? #590

daohu527 opened this issue Oct 9, 2020 · 3 comments · Fixed by #1025
Labels
Milestone

Comments

@daohu527
Copy link

daohu527 commented Oct 9, 2020

I found that SetLogger's severity is int.

void base::SetLogger(LogSeverity severity, base::Logger* logger) {
  MutexLock l(&log_mutex);
  LogDestination::log_destination(severity)->logger_ = logger;
}

but in log_destination severity must in [0, NUM_SEVERITIES]

inline LogDestination* LogDestination::log_destination(LogSeverity severity) {
  assert(severity >=0 && severity < NUM_SEVERITIES);
  if (!log_destinations_[severity]) {
    log_destinations_[severity] = new LogDestination(severity, NULL);
  }
  return log_destinations_[severity];
}

If you use an wrapped logger and set SetLogger's severity to -1, wrapped logger will not work, it seems use the default logger in glog then?

And I found that there are other restrictions in the code that cause severity should be greater or equal to 0. Why not set severity to unsigned type?

@sergiud
Copy link
Collaborator

sergiud commented Jul 14, 2021

Why would you want to set the severity to -1?

@daohu527
Copy link
Author

This maybe a bug, because you can't stop others from setting it up. I actually encountered!

@sergiud sergiud added bug and removed incomplete labels Jul 15, 2021
@sergiud sergiud added this to the 0.7 milestone Jan 3, 2024
@sergiud
Copy link
Collaborator

sergiud commented Jan 3, 2024

With #1025 LogSeverity will become an enum which will prevent implicit conversions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants