On 17 January 2015 at 11:52, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 17 January 2015 at 06:48, Zhang Haoyu <zhan...@sangfor.com.cn> wrote: >> G_IO_OUT|G_IO_HUP are passed from all of the callers >> of chr_add_watch hooker, the assert condition MUST be >> changed. > > "All the callers do this currently" isn't a reason to > change an assert. Possible reasons to change it include: > * "X is required because if we don't then things break because Y" > * "it is meaningless (a programming error) to do X, because Y" > > Can you give the reasoning for changing this assert? > Does it apply also to other add_watch hook functions? > Would the assert be better at a higher level in the callstack?
...that said, I've just figured out what your patch is trying to fix. I think that kind of makes the point about the commit message not really being very clear about why the change is being made, though. (Should we be checking '(cond & G_IO_OUT) != 0'? Is this a bandaid over missing functionality in the spice chardriver?) -- PMM
