Skip to content

Commit

Permalink
ui: drop VNC feature _MASK constants
Browse files Browse the repository at this point in the history 
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.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
  • Loading branch information
@berrange
berrange committed Feb 9, 2024
1 parent 03e471c commit 0e74eb8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 34 deletions.
34 changes: 17 additions & 17 deletions ui/vnc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2144,16 +2144,16 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vs->vnc_encoding = enc;
break;
case VNC_ENCODING_HEXTILE:
vs->features |= VNC_FEATURE_HEXTILE_MASK;
vnc_set_feature(vs, VNC_FEATURE_HEXTILE);
vs->vnc_encoding = enc;
break;
case VNC_ENCODING_TIGHT:
vs->features |= VNC_FEATURE_TIGHT_MASK;
vnc_set_feature(vs, VNC_FEATURE_TIGHT);
vs->vnc_encoding = enc;
break;
#ifdef CONFIG_PNG
case VNC_ENCODING_TIGHT_PNG:
vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
vnc_set_feature(vs, VNC_FEATURE_TIGHT_PNG);
vs->vnc_encoding = enc;
break;
#endif
Expand All @@ -2163,57 +2163,57 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
* So prioritize ZRLE, even if the client hints that it prefers
* ZLIB.
*/
if ((vs->features & VNC_FEATURE_ZRLE_MASK) == 0) {
vs->features |= VNC_FEATURE_ZLIB_MASK;
if (!vnc_has_feature(vs, VNC_FEATURE_ZRLE)) {
vnc_set_feature(vs, VNC_FEATURE_ZLIB);
vs->vnc_encoding = enc;
}
break;
case VNC_ENCODING_ZRLE:
vs->features |= VNC_FEATURE_ZRLE_MASK;
vnc_set_feature(vs, VNC_FEATURE_ZRLE);
vs->vnc_encoding = enc;
break;
case VNC_ENCODING_ZYWRLE:
vs->features |= VNC_FEATURE_ZYWRLE_MASK;
vnc_set_feature(vs, VNC_FEATURE_ZYWRLE);
vs->vnc_encoding = enc;
break;
case VNC_ENCODING_DESKTOPRESIZE:
vs->features |= VNC_FEATURE_RESIZE_MASK;
vnc_set_feature(vs, VNC_FEATURE_RESIZE);
break;
case VNC_ENCODING_DESKTOP_RESIZE_EXT:
vs->features |= VNC_FEATURE_RESIZE_EXT_MASK;
vnc_set_feature(vs, VNC_FEATURE_RESIZE_EXT);
break;
case VNC_ENCODING_POINTER_TYPE_CHANGE:
vs->features |= VNC_FEATURE_POINTER_TYPE_CHANGE_MASK;
vnc_set_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE);
break;
case VNC_ENCODING_RICH_CURSOR:
vs->features |= VNC_FEATURE_RICH_CURSOR_MASK;
vnc_set_feature(vs, VNC_FEATURE_RICH_CURSOR);
break;
case VNC_ENCODING_ALPHA_CURSOR:
vs->features |= VNC_FEATURE_ALPHA_CURSOR_MASK;
vnc_set_feature(vs, VNC_FEATURE_ALPHA_CURSOR);
break;
case VNC_ENCODING_EXT_KEY_EVENT:
send_ext_key_event_ack(vs);
break;
case VNC_ENCODING_AUDIO:
if (vs->vd->audio_state) {
vs->features |= VNC_FEATURE_AUDIO_MASK;
vnc_set_feature(vs, VNC_FEATURE_AUDIO);
send_ext_audio_ack(vs);
}
break;
case VNC_ENCODING_WMVi:
vs->features |= VNC_FEATURE_WMVI_MASK;
vnc_set_feature(vs, VNC_FEATURE_WMVI);
break;
case VNC_ENCODING_LED_STATE:
vs->features |= VNC_FEATURE_LED_STATE_MASK;
vnc_set_feature(vs, VNC_FEATURE_LED_STATE);
break;
case VNC_ENCODING_XVP:
if (vs->vd->power_control) {
vs->features |= VNC_FEATURE_XVP_MASK;
vnc_set_feature(vs, VNC_FEATURE_XVP);
send_xvp_message(vs, VNC_XVP_CODE_INIT);
}
break;
case VNC_ENCODING_CLIPBOARD_EXT:
vs->features |= VNC_FEATURE_CLIPBOARD_EXT_MASK;
vnc_set_feature(vs, VNC_FEATURE_CLIPBOARD_EXT);
vnc_server_cut_text_caps(vs);
break;
case VNC_ENCODING_COMPRESSLEVEL0 ... VNC_ENCODING_COMPRESSLEVEL0 + 9:
Expand Down
22 changes: 5 additions & 17 deletions ui/vnc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,23 +467,6 @@ enum VncFeatures {
VNC_FEATURE_AUDIO,
};

#define VNC_FEATURE_RESIZE_MASK (1 << VNC_FEATURE_RESIZE)
#define VNC_FEATURE_RESIZE_EXT_MASK (1 << VNC_FEATURE_RESIZE_EXT)
#define VNC_FEATURE_HEXTILE_MASK (1 << VNC_FEATURE_HEXTILE)
#define VNC_FEATURE_POINTER_TYPE_CHANGE_MASK (1 << VNC_FEATURE_POINTER_TYPE_CHANGE)
#define VNC_FEATURE_WMVI_MASK (1 << VNC_FEATURE_WMVI)
#define VNC_FEATURE_TIGHT_MASK (1 << VNC_FEATURE_TIGHT)
#define VNC_FEATURE_ZLIB_MASK (1 << VNC_FEATURE_ZLIB)
#define VNC_FEATURE_RICH_CURSOR_MASK (1 << VNC_FEATURE_RICH_CURSOR)
#define VNC_FEATURE_ALPHA_CURSOR_MASK (1 << VNC_FEATURE_ALPHA_CURSOR)
#define VNC_FEATURE_TIGHT_PNG_MASK (1 << VNC_FEATURE_TIGHT_PNG)
#define VNC_FEATURE_ZRLE_MASK (1 << VNC_FEATURE_ZRLE)
#define VNC_FEATURE_ZYWRLE_MASK (1 << VNC_FEATURE_ZYWRLE)
#define VNC_FEATURE_LED_STATE_MASK (1 << VNC_FEATURE_LED_STATE)
#define VNC_FEATURE_XVP_MASK (1 << VNC_FEATURE_XVP)
#define VNC_FEATURE_CLIPBOARD_EXT_MASK (1 << VNC_FEATURE_CLIPBOARD_EXT)
#define VNC_FEATURE_AUDIO_MASK (1 << VNC_FEATURE_AUDIO)


/* Client -> Server message IDs */
#define VNC_MSG_CLIENT_SET_PIXEL_FORMAT 0
Expand Down Expand Up @@ -599,6 +582,11 @@ static inline uint32_t vnc_has_feature(VncState *vs, int feature) {
return (vs->features & (1 << feature));
}

static inline void vnc_set_feature(VncState *vs, enum VncFeatures feature)
{
vs->features |= (1 << feature);
}

/* Framebuffer */
void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
int32_t encoding);
Expand Down

0 comments on commit 0e74eb8

Please sign in to comment.