[Spice-devel] [vdagent-win PATCH 12/13] Factor out an utility function to read strings from registry
Christophe Fergeau
cfergeau at redhat.com
Mon May 28 14:02:02 UTC 2018
On Mon, May 28, 2018 at 09:58:11AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, May 28, 2018 at 09:58:05AM +0100, Frediano Ziglio wrote:
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > > vdagent/display_setting.cpp | 97 +++++++++++++++++++++----------------
> > > 1 file changed, 54 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/vdagent/display_setting.cpp b/vdagent/display_setting.cpp
> > > index 5585bb5..cdc42c7 100644
> > > --- a/vdagent/display_setting.cpp
> > > +++ b/vdagent/display_setting.cpp
> > > @@ -282,30 +282,70 @@ bool DisplaySetting::disable_wallpaper()
> > > }
> > > }
> > >
> > > -bool DisplaySetting::reload_wallpaper(HKEY desktop_reg_key)
> > > +template <typename T>
> > > +static bool RegReadString(HKEY key, const T *name, T *buffer, size_t
> > > buffer_len)
> > > {
> > > - TCHAR wallpaper_path[MAX_PATH + 1];
> > > - DWORD value_size = sizeof(wallpaper_path) - sizeof(wallpaper_path[0]);
> > > + static_assert(sizeof(T) == sizeof(char) || sizeof(T) ==
> > > sizeof(WCHAR));
> > > + DWORD value_size = buffer_len * sizeof(buffer[0]);
> > > DWORD value_type;
> > > LONG status;
> > > - TCHAR cur_wallpaper[MAX_PATH + 1];
> > >
> > > - vd_printf("");
> > > - status = RegQueryValueEx(desktop_reg_key, TEXT("Wallpaper"), NULL,
> > > - &value_type, (LPBYTE)wallpaper_path,
> > > &value_size);
> > > + if (sizeof(T) == sizeof(char)) {
> > > + status = RegQueryValueExA(key, (const char*) name, NULL,
> > > + &value_type, (LPBYTE)buffer,
> > > &value_size);
> > > + } else {
> > > + status = RegQueryValueExW(key, (LPCWSTR) name, NULL,
> > > + &value_type, (LPBYTE)buffer,
> > > &value_size);
> > > + }
> >
> > I think switching win_anim_value/smooth_value to TCHAR/_tcscmp would
> > allow to use RegQueryValueEx in all cases and make this helper a bit
> > simpler?
> >
>
> Does not work with templates, the _tcs macros (and also RegQueryValueEx)
> are replaced with a single instance.
I'm suggesting something like the diff below as a preliminary patch,
I believe with it TCHAR/_tcscmp/RegQueryValueEx would be ok even if they
are replaced by a single instance
diff --git a/vdagent/display_setting.cpp b/vdagent/display_setting.cpp
index 25a248e..1c8cb4e 100644
--- a/vdagent/display_setting.cpp
+++ b/vdagent/display_setting.cpp
@@ -338,7 +338,7 @@ bool DisplaySetting::disable_font_smoothing()
bool DisplaySetting::reload_font_smoothing(HKEY desktop_reg_key)
{
- CHAR smooth_value[4];
+ TCHAR smooth_value[4];
DWORD value_size = sizeof(smooth_value);
DWORD value_type;
LONG status;
@@ -361,10 +361,10 @@ bool DisplaySetting::reload_font_smoothing(HKEY desktop_reg_key)
smooth_value[value_size] = '\0';
}
- if (strcmp(smooth_value, "0") == 0) {
+ if (_tcscmp(smooth_value, "0") == 0) {
vd_printf("font smoothing is disabled in registry. do nothing");
return true;
- } else if (strcmp(smooth_value, "2") != 0) {
+ } else if (_tcscmp(smooth_value, "2") != 0) {
vd_printf("unexpectd font smoothing value %s", smooth_value);
return false;
}
@@ -411,7 +411,7 @@ bool DisplaySetting::disable_animation()
bool DisplaySetting::reload_win_animation(HKEY desktop_reg_key)
{
HKEY win_metrics_hkey;
- CHAR win_anim_value[4];
+ TCHAR win_anim_value[4];
DWORD value_size = sizeof(win_anim_value);
DWORD value_type;
LONG status;
@@ -445,10 +445,10 @@ bool DisplaySetting::reload_win_animation(HKEY desktop_reg_key)
win_anim_value[value_size] = '\0';
}
- if (!strcmp(win_anim_value, "0")) {
+ if (!_tcscmp(win_anim_value, "0")) {
vd_printf("window animation is disabled in registry. do nothing");
return true;
- } else if (strcmp(win_anim_value, "1") != 0) {
+ } else if (_tcscmp(win_anim_value, "1") != 0) {
vd_printf("unexpectd window animation value %s", win_anim_value);
return false;
}
Christophe
>
> > Christophe
> >
> > > + if (status == ERROR_SUCCESS && value_type == REG_SZ) {
> > > + // assure NUL-terminated
> > > + value_size /= sizeof(buffer[0]);
> > > + if (value_size == buffer_len) {
> > > + // full buffer but not terminated?
> > > + if (buffer[value_size-1] != '\0') {
> > > + status = ERROR_MORE_DATA;
> > > + }
> > > + } else {
> > > + // append a NUL. If there's already a NUL character this
> > > + // new one will be ignored
> > > + buffer[value_size] = '\0';
> > > + }
> > > + }
> > > if (status != ERROR_SUCCESS) {
> > > - vd_printf("RegQueryValueEx(Wallpaper) : fail %ld", status);
> > > + if (sizeof(T) == sizeof(char)) {
> > > + vd_printf("RegQueryValueEx(%s) : fail %ld", (const char*)
> > > name, status);
> > > + } else {
> > > + vd_printf("RegQueryValueEx(%ls) : fail %ld", (LPCWSTR) name,
> > > status);
> > > + }
> > > return false;
> > > }
> > >
> > > if (value_type != REG_SZ) {
> > > - vd_printf("bad wallpaper value type %lu (expected REG_SZ)",
> > > value_type);
> > > + if (sizeof(T) == sizeof(char)) {
> > > + vd_printf("bad %s value type %lu (expected REG_SZ)", (const
> > > char*) name, value_type);
> > > + } else {
> > > + vd_printf("bad %ls value type %lu (expected REG_SZ)",
> > > (LPCWSTR) name, value_type);
> > > + }
> > > return false;
> > > }
> > >
> > > - value_size /= sizeof(wallpaper_path[0]);
> > > - if (!value_size || wallpaper_path[value_size - 1] != '\0') {
> > > - wallpaper_path[value_size] = '\0';
> > > + return true;
> > > +}
> > > +
> > > +template <typename T, size_t N>
> > > +static inline bool RegReadString(HKEY key, const T *name, T (&buffer)[N])
> > > +{
> > > + return RegReadString(key, name, buffer, N);
> > > +}
> > > +
> > > +bool DisplaySetting::reload_wallpaper(HKEY desktop_reg_key)
> > > +{
> > > + TCHAR wallpaper_path[MAX_PATH + 1];
> > > + TCHAR cur_wallpaper[MAX_PATH + 1];
> > > +
> > > + vd_printf("");
> > > + if (!RegReadString(desktop_reg_key, TEXT("Wallpaper"),
> > > wallpaper_path)) {
> > > + return false;
> > > }
> > >
> > > if (SystemParametersInfo(SPI_GETDESKWALLPAPER,
> > > SPICE_N_ELEMENTS(cur_wallpaper), cur_wallpaper, 0)) {
> > > @@ -340,28 +380,13 @@ bool DisplaySetting::disable_font_smoothing()
> > > bool DisplaySetting::reload_font_smoothing(HKEY desktop_reg_key)
> > > {
> > > CHAR smooth_value[4+1];
> > > - DWORD value_size = sizeof(smooth_value)-1;
> > > - DWORD value_type;
> > > - LONG status;
> > > BOOL cur_font_smooth;
> > >
> > > vd_printf("");
> > > - status = RegQueryValueExA(desktop_reg_key, "FontSmoothing", NULL,
> > > - &value_type, (LPBYTE)smooth_value,
> > > &value_size);
> > > - if (status != ERROR_SUCCESS) {
> > > - vd_printf("RegQueryValueEx(FontSmoothing) : fail %ld", status);
> > > - return false;
> > > - }
> > > -
> > > - if (value_type != REG_SZ) {
> > > - vd_printf("bad font smoothing value type %lu (expected REG_SZ)",
> > > value_type);
> > > + if (!RegReadString(desktop_reg_key, "FontSmoothing", smooth_value)) {
> > > return false;
> > > }
> > >
> > > - if (!value_size || smooth_value[value_size - 1] != '\0') {
> > > - smooth_value[value_size] = '\0';
> > > - }
> > > -
> > > if (strcmp(smooth_value, "0") == 0) {
> > > vd_printf("font smoothing is disabled in registry. do nothing");
> > > return true;
> > > @@ -413,8 +438,6 @@ bool DisplaySetting::reload_win_animation(HKEY
> > > desktop_reg_key)
> > > {
> > > HKEY win_metrics_hkey;
> > > CHAR win_anim_value[4+1];
> > > - DWORD value_size = sizeof(win_anim_value)-1;
> > > - DWORD value_type;
> > > LONG status;
> > > ANIMATIONINFO active_win_animation;
> > >
> > > @@ -427,25 +450,13 @@ bool DisplaySetting::reload_win_animation(HKEY
> > > desktop_reg_key)
> > > return false;
> > > }
> > >
> > > - status = RegQueryValueExA(win_metrics_hkey, "MinAnimate", NULL,
> > > - &value_type, (LPBYTE)win_anim_value,
> > > &value_size);
> > > - if (status != ERROR_SUCCESS) {
> > > - vd_printf("RegQueryValueEx(MinAnimate) : fail %ld", status);
> > > + if (!RegReadString(win_metrics_hkey, "MinAnimate", win_anim_value)) {
> > > RegCloseKey(win_metrics_hkey);
> > > return false;
> > > }
> > >
> > > RegCloseKey(win_metrics_hkey);
> > >
> > > - if (value_type != REG_SZ) {
> > > - vd_printf("bad MinAnimate value type %lu (expected REG_SZ)",
> > > value_type);
> > > - return false;
> > > - }
> > > -
> > > - if (!value_size || win_anim_value[value_size - 1] != '\0') {
> > > - win_anim_value[value_size] = '\0';
> > > - }
> > > -
> > > if (!strcmp(win_anim_value, "0")) {
> > > vd_printf("window animation is disabled in registry. do nothing");
> > > return true;
>
> Frediano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180528/008bce22/attachment.sig>
More information about the Spice-devel
mailing list