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

Squelch some debug messages when debug_level = 0 #54

Closed
wants to merge 1 commit into from

Conversation

embray
Copy link
Collaborator

@embray embray commented Apr 21, 2017

Protect debugging message with a cysigs.debug_level check. This message breaks one or two tests in Sage when built with debug enabled. I don't know what debug_level it should be at but at least 1, I assume.

Protect debugging message with a `cysigs.debug_level` check.  This message breaks one or two tests in Sage when built with debug enabled.  I don't know what debug_level it should be at but at least 1, I assume.
@jdemeyer
Copy link
Collaborator

This message breaks one or two tests in Sage

Those are genuine bugs.

Your patch is shooting the messenger instead of accepting the message.

@jdemeyer jdemeyer closed this Apr 21, 2017
@embray
Copy link
Collaborator Author

embray commented Apr 21, 2017

If that's a genuine error then should that code be bracketed with #ENABLE_DEBUG_CYSIGNALS in the first place then? Because without debug I don't get the message at all and the code in question (seems to) work fine. So which is it?

@jdemeyer
Copy link
Collaborator

should that code be bracketed with #ENABLE_DEBUG_CYSIGNALS in the first place then?

Good question. I guess I mainly did it for performance reasons. It is reasonably common practice to enable assertions only in debug mode. Think of this code as an assertion check.

Anyway, the underlying issue is #55.

@embray
Copy link
Collaborator Author

embray commented Apr 21, 2017

Right, so I think it's reasonable to be able to disable it. Here you have _sig_off_warning which doesn't check cysigs.debug_level but is always compiled regardless of #ENABLE_DEBUG_CYSIGNALS. Obviously what it's warning about is a programming error that should be warned about, but it seems inconsistent...

@jdemeyer
Copy link
Collaborator

The reason for the difference is that sig_off() is much more common than sig_block(). So the chance that somebody misuses sig_off() is much larger than the chance that somebody misuses sig_block() (just because the chance that person P misuses X is bounded by the chance that P uses X).

@embray
Copy link
Collaborator Author

embray commented Apr 21, 2017

Fair enough. I just though it was surprising that I would get this warning with debug on, but not with debug off.

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.

2 participants