[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #967: sched/semaphore/sem_holder.c: Remove DEBUGPANIC

[GitBox <gi...@apache.org>]

patacongo edited a comment on pull request #967:
URL: https://github.com/apache/incubator-nuttx/pull/967#issuecomment-623489442


   > 
   > 
   > This makes sense to me.
   > 
   > Should we also remove the first line of the assert in `nxsem_restorebaseprio` that was disabled and re-enable it. I think this explains the behavior. I think the second half of the assert is correct.
   
   I removed the code because I don't like commented out code.  It just leaves questions for the next person who deals with the code (and no one to answer the questions).  It is always better to be courageous and remove wrong code.
   
   I did re-enable the debug output because I think it still has valid information.  I dropped from the error to a warning.  I think the output has value if you are debugging with scheduler debug turned off.. although perhaps.  It is not a error any more.  A warning is for unusual events that you might want to know about while debugging.  info() is just chit-char, flow of control stuff.
   
   So generally, my preference would be to leave the warning in place because it provides information for people who are silly enough to use scheduler debug output.  I really doesn't work.  If you enable the scheduler debug output, the system becomes very unstable.  You can't really use the OS to monitor itself in the that way.  syslog output also requires the scheduler.
   
   But I don't mind removing one or both debug output lines if you have strong feelings.  Please confirm if you have such strong feelings and I will update the PR.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org