Jan Kiszka <jan.kis...@web.de> writes: > Luiz, > > I missed this when the API was first proposed: > > cur_mon is scheduled for removal (one day...). It's just an intermediate > step to convert all users to explicit 'mon' passing. Thus, new APIs > should not rely it. > > I just realized that monitor_cur_is_qmp() does so. It should be > refactored to monitor_is_qmp(Monitor *mon). And qerror should be enhance > by a 'mon' argument as well. Callers that aren't passed a 'mon' > themselves should either be fixed at this chance or could fall back to > cur_mon for the time being. > > So far for the theory - do you see any pitfalls in the existing usage?
I put in the new uses of cur_mon, Luiz "only" ACKed them. At any point in the program execution, we have one current monitor, or none. Passing around the current monitor within monitor code is workable, if somewhat tedious. But we need it not just in monitor code, we need it anywhere where we report errors. In other words, pretty much everywhere. Including places that do not and should not know about the monitor. Handing a monitor parameter down pretty much every call chain is beyond tedious, it's impractical. The code reporting an error generally does not and should not know anything about *how* the error gets communicated to the user. Insulating it from that detail is proper separation of concerns, and global variable cur_mon is my tool to get it. Good software engineering. Like many powerful tools, global variables should be used sparingly and with care. I feel this use is well justified. Instead of eliminating cur_mon, I'd like it to be hidden within monitor.c. There are a few uses left outside it.
