Re: [PATCH] ui: drop VNC feature _MASK constants

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 3/1/24 13:26, Daniel P. Berrangé wrote:
> Each VNC feature enum entry has a corresponding _MASK constant
> which is the bit-shifted value. It is very easy for contributors
> to accidentally use the _MASK constant, instead of the non-_MASK
> constant, or the reverse. No compiler warning is possible and
> it'll just silently do the wrong thing at runtime.
> 
> By introducing the vnc_set_feature helper method, we can drop
> all the _MASK constants and thus prevent any future accidents.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   ui/vnc.c | 34 +++++++++++++++++-----------------
>   ui/vnc.h | 21 ++++-----------------
>   2 files changed, 21 insertions(+), 34 deletions(-)


> @@ -599,6 +582,10 @@ static inline uint32_t vnc_has_feature(VncState *vs, int feature) {
>       return (vs->features & (1 << feature));
>   }
>   
> +static inline void vnc_set_feature(VncState *vs, int feature) {

Even stricter using s/int/VncFeatures/ enum type.

With that:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +    vs->features |= (1 << feature);
> +}