Re: [Qemu-devel] [PATCH] spice-char: fix wrong assert condition

[Peter Maydell]

On 17 January 2015 at 11:52, Peter Maydell <address@hidden> wrote:
> On 17 January 2015 at 06:48, Zhang Haoyu <address@hidden> 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